From f3edbb6530c3266f36d2ea4590f84441c3850918 Mon Sep 17 00:00:00 2001 From: Bryan Woodruff Date: Wed, 26 Aug 2026 15:48:38 -0700 Subject: [PATCH 1/6] docs(kg): design proposal for cross-layer entity linking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Measuring the federated graph PR #6 produces says the join is wrong. Of the 67,263 relations that cross a layer boundary in the real 61-layer estate, all 67,263 exist only because of the (name, type) join — no indexer writes a relation across databases, so the union is genuinely disconnected and every bridge is manufactured. The identifiers manufacturing them are Foundation, CodingKeys, print, map, forEach: Swift and JS boilerplate that evades the --join-max-layers guard by appearing in two or three layers rather than sixty. The proposal is a per-type join policy (names identify packages, not functions) plus real derived DEPENDS_ON edges from import→package resolution, with a measurement gate before implementation because it is not yet known whether that resolution finds anything. Rollup is deliberately deferred to a follow-up: aggregating over manufactured edges would only make them look authoritative. --- docs/kg-graph-linking-design.md | 194 ++++++++++++++++++++++++++++++++ 1 file changed, 194 insertions(+) create mode 100644 docs/kg-graph-linking-design.md diff --git a/docs/kg-graph-linking-design.md b/docs/kg-graph-linking-design.md new file mode 100644 index 0000000..0eefb6e --- /dev/null +++ b/docs/kg-graph-linking-design.md @@ -0,0 +1,194 @@ +# Cross-layer entity linking for `kg graph` — Design Proposal + +**Status:** proposal (not yet implemented) · **Date:** 2026-08-26 · **Follows:** PR #5 (`kg graph`), PR #6 (`--federated`) + +How to make a federated entity-relationship graph say something true. `--federated` +(PR #6) merges a scope and its layers into one graph; this proposal fixes *what +connects them*, because the current rule manufactures edges that nobody recorded. + +## Problem + +A federated render is meant to show how entities across an estate relate. Measured +against the real 61-layer estate at `~/Projects` — 667,858 entities, 1,223,605 +relations — it does not: + +| Measurement | Value | +|---|---| +| Relations crossing a layer boundary | 67,263 (5.5%) | +| …of those, existing **only** because of the `(name, type)` join | **67,263 (100%)** | +| …involving `node_modules` | 63 (0.1%) | + +Not one cross-layer relation was written by an indexer. That is not a bug in the +data: relations are stored per-database, and nothing indexes two repositories into +one database, so the federated union is genuinely 61 disconnected components. Every +bridge in the current picture is produced by the join. + +And the joins producing them are not shared symbols. The identifiers generating the +most crossings: + +``` +Foundation 1567 print 528 forEach 227 +CodingKeys 1079 Kr 478 models 221 +createDataFrame 539 map 294 hasError 300 +``` + +Swift and JavaScript boilerplate. They evade the existing `--join-max-layers 3` +guard precisely because they appear in two or three layers rather than sixty. + +**The shape of the failure:** a name-identity join is right for *coarse* entities +whose names are chosen to be globally meaningful (packages, modules, services) and +wrong for *fine* ones whose names are local (functions, fields, generated types). +The current rule applies it to everything. + +## Goals + +- **Cross-layer edges that correspond to something real** — a dependency that exists + in the source, not a name coincidence. +- **A join policy that is per-type and defensible**, defaulting to off for the entity + kinds where names are local. +- **No loss of the honest parts of PR #6** — duplicate-implementation discovery + (six `AddressClient` definitions in three layers) stays; it is just no longer + allowed to imply a call graph. +- **Measurable acceptance**: after the change, the number of cross-layer relations + that survive should be justifiable edge by edge. + +## Non-goals + +- **Inferring dependencies from source.** No new parsing; this works from what the + indexers already record. +- **Rollup/aggregation.** Deferred deliberately — see [Follow-up](#follow-up). + Aggregating over false edges launders the artifact rather than fixing it. +- **Changing single-scope rendering.** PR #5 behaviour is untouched. + +## Design + +### 1. Per-type join policy + +Replace the single `--join-max-layers` guard with a policy keyed on entity type. + +| Entity type | Join across layers | Why | +|---|---|---| +| `package` | **yes** | A package name is chosen to be globally unique; that is what a package name is *for*. | +| `import` | **yes** | An import names something outside the current repo by design. | +| `file` | no | Path-derived and repo-relative; `src/index.ts` is not one file. | +| `type` | opt-in | Sometimes meaningful (`AddressClient`), often not (`CodingKeys`). Off by default. | +| `function` | **no** | `print`, `map`, `forEach`. This is the type generating the false edges. | +| `topic` | no | Documentation headings — the boilerplate case already known. | + +The `--join-max-layers` count guard stays as a second filter on the types that do +join, since even a package name can turn out to be generic. + +`--join-types file,function` overrides the defaults for anyone who wants the old +behaviour, and `--join-types none` disables joining entirely. + +### 2. Package-identity linking — the real edges + +Joining is identity ("these are the same node"). Dependency is a *relation*, and +the indexers already record enough to recover it: + +- Layer A holds an `import` entity named `@depop/foo` (or `com.depop.foo`). +- Layer B holds a `package` entity named `@depop/foo`. + +That is a genuine cross-repo dependency, and it becomes an explicit relation rather +than a fused node: + +``` +(file in A) --IMPORTS--> (import:@depop/foo in A) ==> A's file --DEPENDS_ON--> B's package +``` + +Rules: + +- Match on exact package name. No prefix or fuzzy matching in v1. +- The synthesised edge is typed `DEPENDS_ON` and marked as **derived**, so a reader + can tell it from a relation an indexer wrote. Derived edges are dashed in mermaid + and DOT. +- Self-links (an import resolving to a package in the same layer) are dropped — + that structure is already in the layer's own graph. +- Ambiguity (the same package name defined in two layers) is reported and skipped, + not guessed. + +**This needs measuring before implementation.** The estate holds 5,996 `package` +entities and 66,878 `import` entities; how many imports resolve to a package in +another layer is unknown, and if the answer is near zero the feature is not worth +building as specified. That measurement is the first task, not the first commit. + +### 3. Provenance on every edge + +`GraphEdge` gains: + +```go +// Derived marks an edge synthesised by cross-layer linking rather than read +// from a database. A reader must be able to tell the difference. +Derived bool `json:"derived,omitempty"` +``` + +Renderers draw derived edges dashed. JSON carries the flag. The header comment +counts them: `%% kg graph: 412 nodes, 690 relations (37 derived)`. + +## CLI surface + +```bash +kg graph --federated --root @depop/some-lib --depth 2 # who depends on this library +kg graph --federated --join-types package,import,type # widen the join +kg graph --federated --join-types none # union with no bridges at all +kg graph --federated --no-derived # only relations an indexer wrote +``` + +| Flag | Default | Effect | +|---|---|---| +| `--join-types` | `package,import` | Entity types eligible for cross-layer identity join | +| `--join-max-layers` | `3` | Unchanged; a second filter on the eligible types | +| `--no-derived` | off | Suppress synthesised `DEPENDS_ON` edges | + +## Implementation sketch + +- `internal/knowledge/graph_federated.go`: `joinable` becomes a per-type policy + rather than a single count threshold. +- New `internal/knowledge/graph_link.go`: `LinkPackages(g *Graph) []GraphEdge` — a + pure function over the merged graph, so it is testable without databases, in + keeping with the rest of the graph code. +- `FederationReport` gains `DerivedEdges int` and `AmbiguousPackages []string`. +- Renderers: dashed styling for `Derived`. + +## Test plan + +- Per-type join policy: a `function` named `print` in three layers stays three nodes; + a `package` named `@depop/foo` in two layers becomes one. +- Package linking: an import in layer A resolving to a package in layer B produces + one derived `DEPENDS_ON`; a same-layer resolution produces none; an ambiguous + package name produces none and is reported. +- Derived edges survive JSON round-trip and render dashed in both formats. +- Regression: the estate's top false-edge generators (`Foundation`, `CodingKeys`, + `print`) produce zero cross-layer relations under the new defaults. + +## Acceptance + +Re-run the measurement above. The success condition is not "more edges" — it is that +**every surviving cross-layer relation can be traced to a package an import names**, +and the count of name-coincidence edges is zero under default settings. + +## Follow-up + +Once edges are real, aggregation becomes worth building — `--rollup layer|package`, +collapsing the graph to a granularity a person can read (tens of nodes, weighted +edges), with `--limit` capping groups rather than entities. Aggregating today's +graph would only make the artifacts look authoritative, which is why it is second +and not first. + +## Open questions + +1. **Does package linking find anything?** See the measurement gate in §2. If + cross-layer import→package resolution is rare, the alternative is to accept that + layers are disconnected and make `--federated` a *comparison* tool (find the same + name across repos) rather than a connection tool. +2. **Naming conventions across ecosystems.** `@depop/foo` (npm), `com.depop.foo` + (JVM), and a bare Go module path are three different shapes; v1 matches exact + names only, which may under-match on JVM layers. +3. **Should `type` join by default?** `AddressClient` argues yes, `CodingKeys` argues + no. Proposal: off, revisited with data once §2 is measured. + +## See also + +- [kg-cli-reference.md](kg-cli-reference.md#kg-graph) — the shipped command +- PR #5 — `kg graph` +- PR #6 — `--federated`, whose join this proposal corrects From 4c80b9132f1423a95ffecfee3031feea174c604d Mon Sep 17 00:00:00 2001 From: Bryan Woodruff Date: Thu, 27 Aug 2026 19:32:11 -0700 Subject: [PATCH 2/6] =?UTF-8?q?docs(kg):=20measurement=20gate=20results=20?= =?UTF-8?q?=E2=80=94=20prefix=20matching,=20not=20exact?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gate the proposal put before its own implementation, run against the 61-layer estate. It changed the design rather than confirming it. Exact package-name matching, which the spec proposed for v1, fails: 71 cross-layer hits, and they are api/auth/client/clients — the boilerplate problem one level up. Longest-dotted-prefix matching with a three-segment minimum finds 845 unambiguous cross-layer dependencies, and they are real: services importing com.depop.auth.client resolving to the package in libraries. Layer pairs come out as a plausible dependency map. Slash-separated ecosystems yield exactly zero, and the reason is upstream: not one of the 5,138 package entity names contains a "/", so npm and Go imports have nothing to resolve against. Recorded as an indexer follow-up rather than worked around here. Expected yield: ~845 derived edges replacing 67,263 manufactured ones. --- docs/kg-graph-linking-design.md | 86 ++++++++++++++++++++++++++------- 1 file changed, 68 insertions(+), 18 deletions(-) diff --git a/docs/kg-graph-linking-design.md b/docs/kg-graph-linking-design.md index 0eefb6e..f798bd7 100644 --- a/docs/kg-graph-linking-design.md +++ b/docs/kg-graph-linking-design.md @@ -1,6 +1,6 @@ # Cross-layer entity linking for `kg graph` — Design Proposal -**Status:** proposal (not yet implemented) · **Date:** 2026-08-26 · **Follows:** PR #5 (`kg graph`), PR #6 (`--federated`) +**Status:** proposal (not yet implemented; measurement gate run and passed) · **Date:** 2026-08-26 · **Follows:** PR #5 (`kg graph`), PR #6 (`--federated`) How to make a federated entity-relationship graph say something true. `--federated` (PR #6) merges a scope and its layers into one graph; this proposal fixes *what @@ -96,21 +96,57 @@ than a fused node: (file in A) --IMPORTS--> (import:@depop/foo in A) ==> A's file --DEPENDS_ON--> B's package ``` +**Matching rule (set by measurement — see below): longest namespace-prefix match.** +An import resolves to the longest `package` name that is a dotted prefix of it, with +a minimum of three segments so that `com.depop` cannot claim every JVM import in the +estate. + Rules: -- Match on exact package name. No prefix or fuzzy matching in v1. +- Longest-prefix wins; minimum three segments; ties are impossible by construction. +- **Ambiguity is skipped, not guessed.** If the matched package name is defined in + more than one layer, no edge is drawn and the name is reported. This is not a rare + path: it discards 74% of otherwise-matching imports (below). - The synthesised edge is typed `DEPENDS_ON` and marked as **derived**, so a reader can tell it from a relation an indexer wrote. Derived edges are dashed in mermaid and DOT. - Self-links (an import resolving to a package in the same layer) are dropped — that structure is already in the layer's own graph. -- Ambiguity (the same package name defined in two layers) is reported and skipped, - not guessed. -**This needs measuring before implementation.** The estate holds 5,996 `package` -entities and 66,878 `import` entities; how many imports resolve to a package in -another layer is unknown, and if the answer is near zero the feature is not worth -building as specified. That measurement is the first task, not the first commit. +### Measurement gate — result + +Run against the 61-layer estate (5,138 distinct package names, 50,993 distinct import +names) before writing any of the above. The gate was: does import→package resolution +find anything real? + +| Rule | Cross-layer matches | Unambiguous | +|---|---|---| +| Exact name match (the original v1 proposal) | 71 | 25 | +| Dotted prefix, min 3 segments | 3,204 | **845** | +| Slash prefix (npm / Go), min 2 segments | 0 | 0 | + +**Exact matching fails.** Its 71 hits are generic package fragments — `api`, `auth`, +`client`, `clients`, `app` — and `clients` alone is "defined in" 40 layers. It +reproduces the boilerplate problem one level up. + +**Prefix matching passes, and finds real dependencies:** + +``` +com.depop.auth.client.AccessToken imported in [ads, attribution] -> package com.depop.auth.client in [libraries] +com.depop.auth.client.AuthClient imported in [martech, user] -> package com.depop.auth.client in [libraries] +com.depop.auth.client.ClientCredentials imported in [engage, feature] -> package com.depop.auth.client in [libraries] +``` + +Services depending on the shared auth client. The resulting layer pairs are a +plausible dependency map rather than a name-coincidence map: `libraries—mobileapi` +(174), `clients—product` (151), `libraries—product` (102), `libraries—search` (83), +`libraries—user` (80). + +**Slash-separated ecosystems get nothing, and the reason is upstream.** Not one of +the 5,138 `package` entity names contains a `/` — the indexers never mint package +entities for npm or Go module paths, so the 8,690 slash-style import names have +nothing to resolve against. This is a limitation of what is indexed, not of the +matching rule; see [Follow-up](#follow-up). ### 3. Provenance on every edge @@ -163,12 +199,22 @@ kg graph --federated --no-derived # only relations an inde ## Acceptance -Re-run the measurement above. The success condition is not "more edges" — it is that +Re-run the measurement. The success condition is not "more edges" — it is that **every surviving cross-layer relation can be traced to a package an import names**, and the count of name-coincidence edges is zero under default settings. +Expected yield on the estate: roughly **845 derived `DEPENDS_ON` edges**, replacing +67,263 manufactured ones. Two orders of magnitude fewer edges, and each one +explicable. + ## Follow-up +**Package entities for npm and Go.** The measurement shows the indexers mint +`package` entities only for dotted namespaces. Reading `package.json` `name` fields +and `go.mod` module paths would extend cross-layer linking to the web, client and +tooling layers, which today get no derived edges at all. That is an indexer change, +independent of this proposal and probably larger than it. + Once edges are real, aggregation becomes worth building — `--rollup layer|package`, collapsing the graph to a granularity a person can read (tens of nodes, weighted edges), with `--limit` capping groups rather than entities. Aggregating today's @@ -177,15 +223,19 @@ and not first. ## Open questions -1. **Does package linking find anything?** See the measurement gate in §2. If - cross-layer import→package resolution is rare, the alternative is to accept that - layers are disconnected and make `--federated` a *comparison* tool (find the same - name across repos) rather than a connection tool. -2. **Naming conventions across ecosystems.** `@depop/foo` (npm), `com.depop.foo` - (JVM), and a bare Go module path are three different shapes; v1 matches exact - names only, which may under-match on JVM layers. -3. **Should `type` join by default?** `AddressClient` argues yes, `CodingKeys` argues - no. Proposal: off, revisited with data once §2 is measured. +1. ~~Does package linking find anything?~~ **Answered by the gate:** yes, with + prefix matching (845 unambiguous cross-layer edges); no, with exact matching (71, + mostly generic). The matching rule in §2 changed accordingly. +2. ~~Naming conventions across ecosystems.~~ **Answered:** dotted namespaces resolve; + npm and Go produce nothing because no package entity name contains a `/`. Fixing + that is an indexer change — see [Follow-up](#follow-up). +3. **Is 74% ambiguity acceptable?** Prefix matching finds 3,204 cross-layer + resolutions but only 845 survive the "package defined in exactly one layer" rule. + The discarded ones are mostly packages genuinely duplicated across repos. Options: + accept the loss, draw them to all candidate layers with a marker, or rank by layer + priority. Proposal: accept the loss in v1 and report the count. +4. **Should `type` join by default?** `AddressClient` argues yes, `CodingKeys` argues + no. Proposal: off, revisited once derived edges exist and can be compared against. ## See also From 352fbe46484924854002551cd4667271b4ada98b Mon Sep 17 00:00:00 2001 From: Bryan Woodruff Date: Thu, 27 Aug 2026 20:11:12 -0700 Subject: [PATCH 3/6] feat(kg): per-type join policy and derived cross-layer package links (#10) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(kg): per-type join policy and derived cross-layer package links Implements docs/kg-graph-linking-design.md. The join is now keyed on entity type rather than on a count alone. Names identify packages and imports across repositories because that is what those names are for; they do not identify functions, files or documentation headings. Measured before this change, every one of the 67,263 cross-layer relations in the estate came from the latter kind — Foundation, CodingKeys, print, map, forEach. DefaultJoinTypes is package and import; --join-types widens or narrows it, and an explicitly empty policy joins nothing. Joining says two rows are the same thing, which is still not a dependency. LinkPackages recovers those from what the indexers already record: an import named com.depop.auth.client.AuthClient resolves to the package com.depop.auth.client in another layer, and becomes a DEPENDS_ON edge. Longest prefix wins, with a three-segment floor so com.depop cannot claim every JVM import in the estate, and an ambiguous target is skipped rather than guessed at — on the estate that discards more matches (3,359) than it keeps (2,525). Derived edges carry a flag and render dashed, with their own line in the header. An inference and a recorded fact must not arrive looking alike. On the estate: 2,525 derived links replacing 67,263 manufactured ones, and `--root package:com.depop.auth.client` now draws the eleven layers that depend on the shared auth library. * feat(kg): --join-types and --no-derived --join-types none has to mean "join nothing" while an unset flag means "the default policy"; joinTypePolicy keeps those apart, and is separated from cobra state so that distinction is testable without driving a command. The federated report now says which types were eligible to join, how many links were derived, and how many imports were skipped as ambiguous with examples — a graph that quietly dropped the majority of its matches would read as complete. * docs(kg): document the join policy and derived links Records the corrected yield figure too. The gate predicted 845 and the implementation produces 2,525; both are right and the units differ — 845 distinct import names, 2,525 import nodes, since a name appears separately in each layer that imports it. Verified by recomputing the rule independently against the shipped output. --- docs/kg-cli-reference.md | 71 +++++- docs/kg-graph-linking-design.md | 14 +- src/kg/graph.go | 53 +++- src/kg/graph_test.go | 27 ++ src/kg/internal/knowledge/graph.go | 6 + src/kg/internal/knowledge/graph_federated.go | 44 ++++ .../knowledge/graph_federated_test.go | 72 +++++- src/kg/internal/knowledge/graph_link.go | 202 +++++++++++++++ src/kg/internal/knowledge/graph_link_test.go | 240 ++++++++++++++++++ src/kg/internal/knowledge/graph_render.go | 24 +- 10 files changed, 738 insertions(+), 15 deletions(-) create mode 100644 src/kg/internal/knowledge/graph_link.go create mode 100644 src/kg/internal/knowledge/graph_link_test.go diff --git a/docs/kg-cli-reference.md b/docs/kg-cli-reference.md index 3df1c50..b1bfa55 100644 --- a/docs/kg-cli-reference.md +++ b/docs/kg-cli-reference.md @@ -233,6 +233,8 @@ Error: "main" matches 2 entities — use an ID instead: | `--federated` | off | Render the scope together with every *local* layer it federates with | | `--layer` | *(all)* | With `--federated`, load only these scopes | | `--join-max-layers` | `3` | With `--federated`, the boilerplate guard (see below) | +| `--join-types` | `package,import` | With `--federated`, entity types whose names identify across layers; `none` joins nothing | +| `--no-derived` | off | With `--federated`, omit derived cross-layer package links | `--rel` constrains what the walk follows, not just what is drawn, so filtering to `CONTAINS` gives the containment tree rather than the same neighbourhood with @@ -279,9 +281,19 @@ a plain union of the layers would be a set of disconnected components. What connects them is joining identities across layers. **The join rule.** Two rows in two databases are the same node when their -`(name, type)` match. That surfaces genuinely shared symbols, and equally -surfaces the same name implemented separately in several services — often the -more interesting answer: +`(name, type)` match **and the type is one whose names identify across +repositories** — by default `package` and `import`. + +That restriction is the whole game. A package name is chosen to be globally +unique; that is what a package name is for. A function name is not: measured on +the estate, joining every type produced 67,263 cross-layer relations, and *all* +of them came from names like `Foundation`, `CodingKeys`, `print`, `map` and +`forEach` meaning different things in different repositories. `--join-types` +widens or narrows the policy; `--join-types none` joins nothing and shows the +layers as the disconnected components they actually are. + +Within an eligible type, joining still surfaces the same name implemented +separately in several services — often the more interesting answer: ``` $ kg graph --federated --layer clients,libraries,mobileapi,payments \ @@ -321,6 +333,59 @@ renamed because two layers minted the same ID for different things. Indexer IDs are repo-relative paths, so `import:..` legitimately means something different in every layer. +#### Derived cross-layer links + +Joining says two rows are the same thing. It cannot say that one layer *depends +on* another — and nothing else can either, because relations live inside a +single database and no indexer writes one across repositories. Left at that, a +federated graph is a set of disconnected components. + +The dependency is recoverable, because the indexers already record both ends: a +layer holds an `import` entity named `com.depop.auth.client.AuthClient`, and +another layer holds a `package` entity named `com.depop.auth.client`. `kg graph +--federated` resolves the first to the second and adds a `DEPENDS_ON` edge, +**marked derived and drawn dashed** so it is never mistaken for a relation an +indexer recorded: + +```bash +kg graph --federated --root package:com.depop.auth.client --depth 1 +``` + +``` +%% kg graph: 12 node(s), 11 relation(s) of 717633 entities in the project +%% 11 of those are derived cross-layer links, drawn dashed + subgraph layer0["libraries"] + n0[["com.depop.auth.client"]]:::kgroot + end + subgraph layer1["ads"] + n1[/"com.depop.auth.client.AccessToken"/] + end + ... + n1 -.->|DEPENDS_ON| n0 +``` + +Eleven layers depending on one shared auth library — the question `--federated` +exists to answer. + +Resolution rules: + +- **Longest prefix wins**, with a **three-segment minimum**. Without the floor, + `com.depop` claims every JVM import in the estate and the graph gains one hub + node instead of a dependency map. +- **Ambiguity is skipped, never guessed.** If the matched package is defined in + more than one layer, no edge is drawn and the count is reported. On the estate + this discards 3,359 imports against 2,525 kept — the majority case, not an + edge case. +- **Same-layer resolutions are dropped**; that structure is already in the + layer's own graph. +- `--no-derived` turns the whole thing off, leaving only recorded relations. + +**Only dotted namespaces resolve.** Not one `package` entity in the measured +estate has a `/` in its name — the indexers do not mint package entities for npm +or Go module paths — so JVM, Scala and Kotlin layers get derived links and +JavaScript and Go layers get none. That is a gap in what is indexed rather than +in the matching, and closing it is an indexer change. + **Cost.** Databases are read one at a time and released, so peak memory is the merged graph plus the largest single layer rather than all of them at once. Measured on a 61-layer estate — 668k entities, 1.2M relations — a full load is diff --git a/docs/kg-graph-linking-design.md b/docs/kg-graph-linking-design.md index f798bd7..8230101 100644 --- a/docs/kg-graph-linking-design.md +++ b/docs/kg-graph-linking-design.md @@ -1,6 +1,6 @@ # Cross-layer entity linking for `kg graph` — Design Proposal -**Status:** proposal (not yet implemented; measurement gate run and passed) · **Date:** 2026-08-26 · **Follows:** PR #5 (`kg graph`), PR #6 (`--federated`) +**Status:** implemented · gate run and passed · **Date:** 2026-08-26 · **Follows:** PR #5 (`kg graph`), PR #6 (`--federated`) How to make a federated entity-relationship graph say something true. `--federated` (PR #6) merges a scope and its layers into one graph; this proposal fixes *what @@ -203,9 +203,15 @@ Re-run the measurement. The success condition is not "more edges" — it is that **every surviving cross-layer relation can be traced to a package an import names**, and the count of name-coincidence edges is zero under default settings. -Expected yield on the estate: roughly **845 derived `DEPENDS_ON` edges**, replacing -67,263 manufactured ones. Two orders of magnitude fewer edges, and each one -explicable. +Yield on the estate, as implemented: **2,525 derived `DEPENDS_ON` edges**, +replacing 67,263 manufactured ones. + +The gate predicted 845, and both numbers are right: 845 is the count of distinct +import *names* that resolve, while 2,525 counts import *nodes* — the same name +appears as a separate node in each layer that imports it, since imports join only +across three layers or fewer. Verified by recomputing the rule independently +against the shipped output: 2,525 = 2,525, and the name count reproduces 845 +exactly. ## Follow-up diff --git a/src/kg/graph.go b/src/kg/graph.go index 3b5c5a5..acf011a 100644 --- a/src/kg/graph.go +++ b/src/kg/graph.go @@ -6,6 +6,7 @@ import ( "io" "os" "path/filepath" + "strings" "github.com/cortexa-llc/mcp/kg/internal/knowledge" "github.com/spf13/cobra" @@ -32,6 +33,8 @@ var ( graphFederated bool graphLayers []string graphMaxJoin int + graphJoinTypes []string + graphNoDerived bool ) // graphSettings is one invocation's flags, kept separate from the flag @@ -90,7 +93,7 @@ Examples: // Silently ignoring these would render one database and look like it // had honoured the request. if !graphFederated { - for _, flag := range []string{"layer", "join-max-layers"} { + for _, flag := range []string{"layer", "join-max-layers", "join-types", "no-derived"} { if cmd.Flags().Changed(flag) { return fmt.Errorf("--%s only applies with --federated", flag) } @@ -259,6 +262,14 @@ func renderLoadedGraph(graph *knowledge.Graph, s graphSettings, out io.Writer) ( } // runFederatedGraph renders across a scope and every layer it federates with. +// ambiguousSuffix names a few of the packages that could not be resolved. +func ambiguousSuffix(names []string) string { + if len(names) == 0 { + return "." + } + return ", e.g. " + strings.Join(names[:min(3, len(names))], ", ") + "." +} + func runFederatedGraph(cmd *cobra.Command, s graphSettings, out io.Writer) (knowledge.Subgraph, error) { if usePersonal { return knowledge.Subgraph{}, fmt.Errorf("--personal and --federated are mutually exclusive: the personal store has no layers") @@ -294,6 +305,8 @@ func runFederatedGraph(cmd *cobra.Command, s graphSettings, out io.Writer) (know ProjectID: filepath.Base(root), OnlyLayers: graphLayers, MaxJoinLayers: graphMaxJoin, + JoinTypes: parseJoinTypes(cmd), + NoDerived: graphNoDerived, }) if err != nil { return knowledge.Subgraph{}, err @@ -303,6 +316,26 @@ func runFederatedGraph(cmd *cobra.Command, s graphSettings, out io.Writer) (know return renderLoadedGraph(graph, s, out) } +// parseJoinTypes turns --join-types into the loader's policy. An unset flag +// means the default policy; "none" means join nothing, which is distinct from +// unset and has to survive as an empty-but-not-nil slice. +func parseJoinTypes(cmd *cobra.Command) []string { + return joinTypePolicy(cmd.Flags().Changed("join-types"), graphJoinTypes) +} + +// joinTypePolicy is parseJoinTypes without the cobra state, so the distinction +// it exists to preserve can be tested directly: nil means "the default policy", +// while an empty-but-not-nil slice means "join nothing". +func joinTypePolicy(changed bool, values []string) []string { + if !changed { + return nil + } + if len(values) == 1 && strings.EqualFold(strings.TrimSpace(values[0]), "none") { + return []string{} + } + return values +} + // printFederationReport writes what the federated load did to stderr, keeping // stdout a clean document. A merged graph hides two things a reader would want // to know — a layer that failed to open, and identities too widespread to @@ -324,6 +357,10 @@ func printFederationReport(cmd *cobra.Command, report *knowledge.FederationRepor } cmd.PrintErrf("Federated %d layer(s): %d node(s), %d relation(s).\n", contributing, nodes, edges) + cmd.PrintErrf("Joining identities across layers for: %s\n", strings.Join(report.JoinTypes, ", ")) + if len(report.JoinTypes) == 0 { + cmd.PrintErrln(" (nothing — layers are shown as the disconnected components they are)") + } if report.Joined > 0 { cmd.PrintErrf("Joined %d identit%s across layers, merging %d duplicate row(s).\n", report.Joined, plural(report.Joined, "y", "ies"), report.MergedNodes) @@ -343,6 +380,16 @@ func printFederationReport(cmd *cobra.Command, report *knowledge.FederationRepor "IDs from repo-relative paths, so the same ID can mean different things in different layers.\n", report.IDCollisions) } + if link := report.Link; link.Derived > 0 || link.Ambiguous > 0 { + cmd.PrintErrf("Derived %d cross-layer package link(s), drawn dashed.\n", link.Derived) + if link.SameLayer > 0 { + cmd.PrintErrf(" %d import(s) resolved inside their own layer and were left alone.\n", link.SameLayer) + } + if link.Ambiguous > 0 { + cmd.PrintErrf(" %d import(s) matched a package defined in more than one layer and were skipped "+ + "rather than guessed at%s\n", link.Ambiguous, ambiguousSuffix(link.AmbiguousNames)) + } + } for _, failed := range report.FailedLayers() { cmd.PrintErrf("Warning: layer %s could not be read and is missing from this graph: %s\n", failed.Name, failed.Failed) @@ -377,6 +424,10 @@ func init() { "Render the scope together with every local layer it federates with, joining shared identities across them (remote hub layers are search-only)") graphCmd.Flags().StringSliceVar(&graphLayers, "layer", nil, "With --federated, restrict the load to these scopes instead of all layers") + graphCmd.Flags().StringSliceVar(&graphJoinTypes, "join-types", knowledge.DefaultJoinTypes, + "With --federated, entity types whose names identify the same thing across layers; \"none\" to join nothing") + graphCmd.Flags().BoolVar(&graphNoDerived, "no-derived", false, + "With --federated, omit derived cross-layer package links, leaving only relations an indexer recorded") graphCmd.Flags().IntVar(&graphMaxJoin, "join-max-layers", knowledge.DefaultMaxJoinLayers, "With --federated, a name found in more than this many layers is boilerplate, not one shared symbol, and is left unjoined") registerPersonalFlag(graphCmd) diff --git a/src/kg/graph_test.go b/src/kg/graph_test.go index 4e21bd5..d22b614 100644 --- a/src/kg/graph_test.go +++ b/src/kg/graph_test.go @@ -220,6 +220,8 @@ func TestRunFederatedGraphValidatesSettingsFirst(t *testing.T) { func TestGraphCommandFederationFlags(t *testing.T) { for _, tc := range []struct{ flag, want string }{ {flag: "federated", want: "false"}, + {flag: "no-derived", want: "false"}, + {flag: "join-types", want: "[" + strings.Join(knowledge.DefaultJoinTypes, ",") + "]"}, {flag: "join-max-layers", want: strconv.Itoa(knowledge.DefaultMaxJoinLayers)}, {flag: "layer", want: "[]"}, } { @@ -234,6 +236,31 @@ func TestGraphCommandFederationFlags(t *testing.T) { } } +// "none" has to survive as an empty-but-not-nil policy: nil means "use the +// default types", and conflating the two would silently join everything the +// user asked to join nothing. +func TestJoinTypePolicy(t *testing.T) { + if got := joinTypePolicy(false, knowledge.DefaultJoinTypes); got != nil { + t.Errorf("unset flag = %v, want nil so the default policy applies", got) + } + + got := joinTypePolicy(true, []string{"none"}) + if got == nil { + t.Fatal("\"none\" produced nil, which means the default policy — it must join nothing") + } + if len(got) != 0 { + t.Errorf("\"none\" = %v, want an empty policy", got) + } + + if got := joinTypePolicy(true, []string{" NONE "}); got == nil || len(got) != 0 { + t.Errorf("case and spacing changed the meaning of \"none\": %v", got) + } + + if got := joinTypePolicy(true, []string{"package", "type"}); strings.Join(got, ",") != "package,type" { + t.Errorf("explicit types = %v, want [package type]", got) + } +} + func commandRegistered(t *testing.T, name string) bool { t.Helper() for _, c := range rootCmd.Commands() { diff --git a/src/kg/internal/knowledge/graph.go b/src/kg/internal/knowledge/graph.go index a8da1bd..8a008da 100644 --- a/src/kg/internal/knowledge/graph.go +++ b/src/kg/internal/knowledge/graph.go @@ -43,6 +43,12 @@ type GraphEdge struct { FromID string `json:"from_id"` ToID string `json:"to_id"` Type string `json:"type"` + + // Derived marks an edge this tool worked out rather than read from a + // database — today, cross-layer package links. A reader has to be able to + // tell the two apart, so renderers draw derived edges dashed and the + // header counts them separately. + Derived bool `json:"derived,omitempty"` } // Graph is a whole project's entities and relations held in memory. diff --git a/src/kg/internal/knowledge/graph_federated.go b/src/kg/internal/knowledge/graph_federated.go index 911b088..b8bed13 100644 --- a/src/kg/internal/knowledge/graph_federated.go +++ b/src/kg/internal/knowledge/graph_federated.go @@ -4,6 +4,7 @@ import ( "fmt" "path/filepath" "sort" + "strings" ) // Federated graph loading — one Graph built from a scope and every layer it @@ -32,6 +33,22 @@ import ( // of places, while template text appears in everything. const DefaultMaxJoinLayers = 3 +// DefaultJoinTypes are the entity types whose names identify the same thing +// across databases. +// +// The distinction is not how widespread a name is but whether names of that +// kind are chosen to be globally meaningful. A package name is: that is what a +// package name is for. A function name is not — measured on a real estate, the +// identifiers producing the most cross-layer joins were Foundation, +// CodingKeys, print, map and forEach, none of which means the same thing in +// two repositories. Files are path-derived, and topics are documentation +// headings. +// +// Types are excluded by default and available through --join-types: some +// genuinely identify (AddressClient), many do not (CodingKeys), and there is +// no way to tell from the name alone. +var DefaultJoinTypes = []string{EntityTypePackage, EntityTypeImport} + // FederatedGraphOptions describes which databases to assemble into one graph. type FederatedGraphOptions struct { // AIDir is the .ai directory holding the scope databases. @@ -45,6 +62,13 @@ type FederatedGraphOptions struct { OnlyLayers []string // MaxJoinLayers is the boilerplate guard; zero means DefaultMaxJoinLayers. MaxJoinLayers int + // JoinTypes are the entity types eligible for cross-layer identity + // joining. Nil means DefaultJoinTypes; an explicitly empty slice joins + // nothing, leaving the layers as the disconnected components they are. + JoinTypes []string + // NoDerived suppresses cross-layer package linking, leaving only relations + // an indexer actually recorded. + NoDerived bool } // LayerLoad is one layer's contribution to a federated graph. @@ -78,6 +102,10 @@ type FederationReport struct { // IDCollisions counts nodes whose ID already existed in another layer and // were renamed to keep them distinct. IDCollisions int `json:"id_collisions,omitempty"` + // JoinTypes are the entity types that were eligible to join. + JoinTypes []string `json:"join_types"` + // Link records what cross-layer package linking derived. + Link LinkReport `json:"link"` } // FailedLayers returns the layers that could not be read. A federated render @@ -132,8 +160,20 @@ func LoadFederatedGraph(opts FederatedGraphOptions) (*Graph, *FederationReport, } } + joinTypes := opts.JoinTypes + if joinTypes == nil { + joinTypes = DefaultJoinTypes + } + eligible := lowerSet(joinTypes) + report.JoinTypes = append([]string{}, joinTypes...) + joinable := make(map[ntKey]bool) for key, count := range layerCount { + if !eligible[strings.ToLower(key.Type)] { + // This type's names are local to a repository; two matches are a + // coincidence, not an identity. + continue + } switch { case count <= 1: // Nothing to join. @@ -273,6 +313,10 @@ func LoadFederatedGraph(opts FederatedGraphOptions) (*Graph, *FederationReport, }) } + if !opts.NoDerived { + report.Link = LinkPackages(merged) + } + // Joined nodes accumulate layers in load order; sorting makes the rendered // provenance stable. for id, node := range merged.nodes { diff --git a/src/kg/internal/knowledge/graph_federated_test.go b/src/kg/internal/knowledge/graph_federated_test.go index 3d29751..6e49e49 100644 --- a/src/kg/internal/knowledge/graph_federated_test.go +++ b/src/kg/internal/knowledge/graph_federated_test.go @@ -57,9 +57,17 @@ func seedLayer(t *testing.T, aiDir, name string, layers []string, entities ...fe } } -// loadFed is the common call under test. +// loadFed is the common call under test, with the default join policy. func loadFed(t *testing.T, aiDir, scopeName string, maxJoin int, only ...string) (*Graph, *FederationReport) { t.Helper() + return loadFedJoining(t, aiDir, scopeName, maxJoin, nil, only...) +} + +// loadFedJoining is loadFed with an explicit join policy. Most fixtures here +// predate the per-type policy and were written around `type` entities, so they +// say so rather than relying on a default that deliberately excludes them. +func loadFedJoining(t *testing.T, aiDir, scopeName string, maxJoin int, joinTypes []string, only ...string) (*Graph, *FederationReport) { + t.Helper() scope, err := LoadScopeConfig(aiDir, scopeName) if err != nil { @@ -71,6 +79,7 @@ func loadFed(t *testing.T, aiDir, scopeName string, maxJoin int, only ...string) ProjectID: fedProject, OnlyLayers: only, MaxJoinLayers: maxJoin, + JoinTypes: joinTypes, }) if err != nil { t.Fatalf("LoadFederatedGraph: %v", err) @@ -110,7 +119,7 @@ func TestLoadFederatedGraphJoinsSharedIdentities(t *testing.T) { fedEntity{"estate.md", EntityTypeFile}, ) - g, report := loadFed(t, aiDir, "estate", 0) + g, report := loadFedJoining(t, aiDir, "estate", 0, []string{EntityTypeType}) // Address exists in two layers and must arrive as one node carrying both. address := nodeByName(t, g, "Address") @@ -162,7 +171,8 @@ func TestLoadFederatedGraphSuppressesWidespreadNames(t *testing.T) { fedEntity{"estate.md", EntityTypeFile}, ) - g, report := loadFed(t, aiDir, "estate", 2) // "Deployment" is in 3 layers + joinTopics := []string{EntityTypeTopic} + g, report := loadFedJoining(t, aiDir, "estate", 2, joinTopics) // "Deployment" is in 3 layers var deployments int for _, n := range g.nodes { @@ -184,7 +194,7 @@ func TestLoadFederatedGraphSuppressesWidespreadNames(t *testing.T) { } // Raising the guard joins it, which is the escape hatch the report offers. - g2, report2 := loadFed(t, aiDir, "estate", 3) + g2, report2 := loadFedJoining(t, aiDir, "estate", 3, joinTopics) if report2.Joined != 1 { t.Errorf("with a raised guard, Joined = %d, want 1", report2.Joined) } @@ -283,7 +293,7 @@ func TestLoadFederatedGraphRenamesCollidingIDs(t *testing.T) { // maxJoin 1 leaves the identity unjoined, so nothing but the ID could fuse // these two. - g, report := loadFed(t, aiDir, "estate", 1) + g, report := loadFedJoining(t, aiDir, "estate", 1, []string{}) var dots []GraphNode for _, n := range g.nodes { @@ -310,6 +320,58 @@ func TestLoadFederatedGraphRenamesCollidingIDs(t *testing.T) { } } +// The join policy is the correction this package exists to make: a name +// identifies a package across repositories and does not identify a function. +func TestLoadFederatedGraphJoinPolicyByType(t *testing.T) { + aiDir := t.TempDir() + for _, layer := range []string{"a", "b"} { + seedLayer(t, aiDir, layer, nil, + fedEntity{layer + ".go", EntityTypeFile}, + fedEntity{"print", EntityTypeFunction}, // never joins + fedEntity{"com.depop.auth", EntityTypePackage}, // joins by default + fedEntity{"CodingKeys", EntityTypeType}, // joins only on request + ) + } + seedLayer(t, aiDir, "estate", []string{"a", "b"}, fedEntity{"estate.md", EntityTypeFile}) + + countNamed := func(g *Graph, name string) int { + n := 0 + for _, node := range g.nodes { + if node.Name == name { + n++ + } + } + return n + } + + g, _ := loadFed(t, aiDir, "estate", 0) + if got := countNamed(g, "print"); got != 2 { + t.Errorf("function \"print\" collapsed to %d node(s); it must never join", got) + } + if got := countNamed(g, "com.depop.auth"); got != 1 { + t.Errorf("package joined into %d node(s), want 1", got) + } + if got := countNamed(g, "CodingKeys"); got != 2 { + t.Errorf("type joined into %d node(s) under the default policy, want 2", got) + } + + // Opting a type in is what --join-types is for. + g2, _ := loadFedJoining(t, aiDir, "estate", 0, []string{EntityTypePackage, EntityTypeType}) + if got := countNamed(g2, "CodingKeys"); got != 1 { + t.Errorf("with type joining requested, got %d node(s), want 1", got) + } + if got := countNamed(g2, "print"); got != 2 { + t.Errorf("function joined when it was not in the policy: %d node(s)", got) + } + + // An explicitly empty policy joins nothing — the layers stay the + // disconnected components they actually are. + g3, _ := loadFedJoining(t, aiDir, "estate", 0, []string{}) + if got := countNamed(g3, "com.depop.auth"); got != 2 { + t.Errorf("empty join policy still joined packages: %d node(s), want 2", got) + } +} + func TestFederationOrder(t *testing.T) { scope := &ScopeConfig{Name: "estate", Layers: []string{"a", "b", "c"}} diff --git a/src/kg/internal/knowledge/graph_link.go b/src/kg/internal/knowledge/graph_link.go new file mode 100644 index 0000000..aa1a686 --- /dev/null +++ b/src/kg/internal/knowledge/graph_link.go @@ -0,0 +1,202 @@ +package knowledge + +import ( + "sort" + "strings" +) + +// Cross-layer package linking — deriving real dependency edges between layers. +// +// Federated layers are genuinely disconnected: relations live inside one +// database and no indexer writes one across repositories. Joining identities +// by name bridges them, but measurement showed that bridge is made of +// coincidences — Foundation, CodingKeys, print, map — and not of dependencies. +// +// A dependency is recoverable without inventing anything, because the indexers +// already record both ends: layer A holds an `import` entity naming +// com.depop.auth.client.AuthClient, and layer B holds a `package` entity named +// com.depop.auth.client. That is a real edge, and it is a relation rather than +// an identity, so it is drawn as one and marked derived. +// +// Measured on a 61-layer estate: exact name matching finds 71 cross-layer +// links, nearly all of them generic fragments like "auth" and "clients". +// Longest-prefix matching with a three-segment floor finds 845 unambiguous +// ones, and they read as a dependency map. See docs/kg-graph-linking-design.md. + +// minPackageSegments is how specific a package name must be before an import +// may resolve to it. Without a floor, "com.depop" claims every JVM import in +// the estate and the graph gains one hub node instead of a dependency map. +const minPackageSegments = 3 + +// packageSeparator is the namespace separator prefix matching understands. +// +// Only dotted namespaces resolve today. Not one package entity in the measured +// estate had a "/" in its name — the indexers do not mint package entities for +// npm or Go module paths — so slash-style imports have nothing to resolve +// against. That is a gap in what is indexed, not in this matching rule. +const packageSeparator = "." + +// LinkReport records what package linking derived, and what it refused to. +type LinkReport struct { + // Derived is the number of DEPENDS_ON edges added. + Derived int `json:"derived"` + // Ambiguous counts imports that matched a package name defined in more + // than one layer. Those are skipped rather than guessed at; on a real + // estate this discards more matches than it keeps. + Ambiguous int `json:"ambiguous"` + // AmbiguousNames samples the package names responsible, worst first. + AmbiguousNames []string `json:"ambiguous_names,omitempty"` + // SameLayer counts imports that resolved to a package in their own layer. + // That structure is already in the layer's own graph, so no edge is added. + SameLayer int `json:"same_layer"` +} + +// packageTarget is one package name's presence across the merged graph. +type packageTarget struct { + // nodeID is the package node an import should point at. Meaningful only + // when the name resolves to a single layer. + nodeID string + layers map[string]bool +} + +// LinkPackages derives DEPENDS_ON edges from imports to the packages they +// name, and adds them to g. Pure with respect to the store: it reads only the +// merged graph, so it is testable without a database. +func LinkPackages(g *Graph) LinkReport { + report := LinkReport{} + targets := packageIndex(g) + if len(targets) == 0 { + return report + } + + ambiguous := map[string]bool{} + + // Imports are visited in ID order so that the derived edges — and the + // samples in the report — are the same on every run. + for _, node := range sortedNodesOfType(g, EntityTypeImport) { + name, target := resolvePackage(node.Name, targets) + if target == nil { + continue + } + + if len(target.layers) > 1 { + report.Ambiguous++ + ambiguous[name] = true + continue + } + + // An import that only ever appears in the layer defining the package + // is describing that layer's own structure, which its graph already + // holds. + if !crossesLayer(node.Layers, target.layers) { + report.SameLayer++ + continue + } + + edge := GraphEdge{ + FromID: node.ID, + ToID: target.nodeID, + Type: RelDependsOn, + Derived: true, + } + if edge.FromID == edge.ToID { + continue + } + g.out[edge.FromID] = append(g.out[edge.FromID], edge) + g.in[edge.ToID] = append(g.in[edge.ToID], edge) + g.edgeCount++ + report.Derived++ + } + + report.AmbiguousNames = make([]string, 0, len(ambiguous)) + for name := range ambiguous { + report.AmbiguousNames = append(report.AmbiguousNames, name) + } + sort.Strings(report.AmbiguousNames) + if len(report.AmbiguousNames) > 10 { + report.AmbiguousNames = report.AmbiguousNames[:10] + } + return report +} + +// packageIndex collects every package entity by name, with the layers it is +// defined in. A name can arrive as several nodes when the join guard kept them +// apart, which is itself a form of ambiguity and is recorded as such. +func packageIndex(g *Graph) map[string]*packageTarget { + targets := make(map[string]*packageTarget) + for _, node := range g.nodes { + if !strings.EqualFold(node.Type, EntityTypePackage) { + continue + } + if segments(node.Name) < minPackageSegments { + continue + } + target, ok := targets[node.Name] + if !ok { + target = &packageTarget{nodeID: node.ID, layers: map[string]bool{}} + targets[node.Name] = target + } + // Lowest node ID wins, so the chosen target does not depend on map + // iteration order. + if node.ID < target.nodeID { + target.nodeID = node.ID + } + for _, layer := range node.Layers { + target.layers[layer] = true + } + if len(node.Layers) == 0 { + // A non-federated graph has no layer provenance; treat the node's + // own presence as one layer so single-database use is well-defined. + target.layers[""] = true + } + } + return targets +} + +// resolvePackage finds the longest package name that prefixes importName, +// returning the name and its target. Longest-first means +// com.depop.auth.client beats com.depop.auth, which is the specific answer. +func resolvePackage(importName string, targets map[string]*packageTarget) (string, *packageTarget) { + parts := strings.Split(importName, packageSeparator) + for cut := len(parts) - 1; cut >= minPackageSegments; cut-- { + candidate := strings.Join(parts[:cut], packageSeparator) + if target, ok := targets[candidate]; ok { + return candidate, target + } + } + return "", nil +} + +// crossesLayer reports whether the import is used anywhere the package is not +// defined — the condition that makes the dependency a cross-layer one. +func crossesLayer(importLayers []string, packageLayers map[string]bool) bool { + if len(importLayers) == 0 { + return false + } + for _, layer := range importLayers { + if !packageLayers[layer] { + return true + } + } + return false +} + +// segments counts the namespace segments in a package name. +func segments(name string) int { + if name == "" { + return 0 + } + return strings.Count(name, packageSeparator) + 1 +} + +// sortedNodesOfType returns every node of one type in ID order. +func sortedNodesOfType(g *Graph, entityType string) []GraphNode { + var nodes []GraphNode + for _, node := range g.nodes { + if strings.EqualFold(node.Type, entityType) { + nodes = append(nodes, node) + } + } + sort.Slice(nodes, func(i, j int) bool { return nodes[i].ID < nodes[j].ID }) + return nodes +} diff --git a/src/kg/internal/knowledge/graph_link_test.go b/src/kg/internal/knowledge/graph_link_test.go new file mode 100644 index 0000000..c367a3f --- /dev/null +++ b/src/kg/internal/knowledge/graph_link_test.go @@ -0,0 +1,240 @@ +package knowledge + +import ( + "sort" + "strings" + "testing" +) + +// linkGraph builds a merged-graph fixture directly: LinkPackages reads only +// nodes and layers, so none of this needs a database. +func linkGraph(nodes ...GraphNode) *Graph { + g := &Graph{ + nodes: make(map[string]GraphNode, len(nodes)), + out: make(map[string][]GraphEdge), + in: make(map[string][]GraphEdge), + } + for _, n := range nodes { + g.nodes[n.ID] = n + } + return g +} + +func pkgNode(id, name string, layers ...string) GraphNode { + return GraphNode{ID: id, Name: name, Type: EntityTypePackage, Layers: layers} +} + +func importNode(id, name string, layers ...string) GraphNode { + return GraphNode{ID: id, Name: name, Type: EntityTypeImport, Layers: layers} +} + +// derivedEdges returns every derived edge as "from -TYPE-> to", sorted. +func derivedEdges(g *Graph) []string { + var out []string + for _, edges := range g.out { + for _, e := range edges { + if e.Derived { + out = append(out, e.FromID+" -"+e.Type+"-> "+e.ToID) + } + } + } + sort.Strings(out) + return out +} + +func TestLinkPackagesDerivesCrossLayerDependency(t *testing.T) { + g := linkGraph( + pkgNode("pkg1", "com.depop.auth.client", "libraries"), + importNode("imp1", "com.depop.auth.client.AuthClient", "martech"), + ) + + report := LinkPackages(g) + + if report.Derived != 1 { + t.Fatalf("Derived = %d, want 1", report.Derived) + } + if got := derivedEdges(g); len(got) != 1 || got[0] != "imp1 -DEPENDS_ON-> pkg1" { + t.Errorf("edges = %v, want [imp1 -DEPENDS_ON-> pkg1]", got) + } + if g.EdgeCount() != 1 { + t.Errorf("EdgeCount = %d, want 1", g.EdgeCount()) + } + // The edge has to be reachable in both directions, or "who depends on this + // library" — the question this feature exists to answer — cannot be asked. + if len(g.in["pkg1"]) != 1 { + t.Errorf("derived edge missing from the incoming adjacency list") + } +} + +// The most specific package that prefixes the import wins: com.depop.auth.client +// tells you which library, com.depop.auth barely narrows it down. +func TestLinkPackagesPrefersTheLongestPrefix(t *testing.T) { + g := linkGraph( + pkgNode("short", "com.depop.auth", "libraries"), + pkgNode("long", "com.depop.auth.client", "libraries"), + importNode("imp1", "com.depop.auth.client.AuthClient", "martech"), + ) + + LinkPackages(g) + + if got := derivedEdges(g); len(got) != 1 || !strings.HasSuffix(got[0], "-> long") { + t.Errorf("edges = %v, want the link to resolve to the longer package", got) + } +} + +// Without a floor on specificity, "com.depop" claims every JVM import in the +// estate and the graph gains one hub node instead of a dependency map. +func TestLinkPackagesIgnoresUnspecificPackages(t *testing.T) { + g := linkGraph( + pkgNode("pkg1", "com.depop", "libraries"), + importNode("imp1", "com.depop.auth.client.AuthClient", "martech"), + ) + + report := LinkPackages(g) + + if report.Derived != 0 { + t.Errorf("Derived = %d, want 0 for a two-segment package", report.Derived) + } +} + +// A package defined in several layers cannot say which one an import meant, and +// guessing would invent exactly the kind of edge this feature replaces. +func TestLinkPackagesSkipsAmbiguousTargets(t *testing.T) { + g := linkGraph( + pkgNode("pkg1", "com.depop.auth.client", "libraries", "tooling"), + importNode("imp1", "com.depop.auth.client.AuthClient", "martech"), + ) + + report := LinkPackages(g) + + if report.Derived != 0 { + t.Errorf("Derived = %d, want 0", report.Derived) + } + if report.Ambiguous != 1 { + t.Errorf("Ambiguous = %d, want 1", report.Ambiguous) + } + if len(report.AmbiguousNames) != 1 || report.AmbiguousNames[0] != "com.depop.auth.client" { + t.Errorf("AmbiguousNames = %v, want [com.depop.auth.client]", report.AmbiguousNames) + } +} + +// The join guard can leave one package name as several nodes; that is the same +// ambiguity arriving by a different route and must be treated the same way. +func TestLinkPackagesTreatsSplitNodesAsAmbiguous(t *testing.T) { + g := linkGraph( + pkgNode("pkgA", "com.depop.auth.client", "libraries"), + pkgNode("pkgB", "com.depop.auth.client", "tooling"), + importNode("imp1", "com.depop.auth.client.AuthClient", "martech"), + ) + + report := LinkPackages(g) + + if report.Derived != 0 || report.Ambiguous != 1 { + t.Errorf("Derived = %d, Ambiguous = %d; want 0 and 1", report.Derived, report.Ambiguous) + } +} + +// An import used only where the package is defined describes that layer's own +// structure, which its graph already holds. +func TestLinkPackagesSkipsSameLayerResolution(t *testing.T) { + g := linkGraph( + pkgNode("pkg1", "com.depop.auth.client", "libraries"), + importNode("imp1", "com.depop.auth.client.AuthClient", "libraries"), + ) + + report := LinkPackages(g) + + if report.Derived != 0 { + t.Errorf("Derived = %d, want 0", report.Derived) + } + if report.SameLayer != 1 { + t.Errorf("SameLayer = %d, want 1", report.SameLayer) + } +} + +// A joined import spanning several layers still depends on the package from the +// layers that are not the defining one. +func TestLinkPackagesLinksWhenAnyLayerIsForeign(t *testing.T) { + g := linkGraph( + pkgNode("pkg1", "com.depop.auth.client", "libraries"), + importNode("imp1", "com.depop.auth.client.AuthClient", "libraries", "martech"), + ) + + report := LinkPackages(g) + + if report.Derived != 1 { + t.Errorf("Derived = %d, want 1 — martech imports it from outside libraries", report.Derived) + } +} + +// Documents a known gap rather than a design choice: no indexer mints package +// entities for npm or Go module paths, so slash-style imports resolve to +// nothing. If that changes upstream, this test is the place it surfaces. +func TestLinkPackagesDoesNotResolveSlashNamespaces(t *testing.T) { + g := linkGraph( + pkgNode("pkg1", "@depop/auth-client", "libraries"), + importNode("imp1", "@depop/auth-client/dist/index", "martech"), + ) + + report := LinkPackages(g) + + if report.Derived != 0 { + t.Errorf("Derived = %d; slash namespaces are not matched today", report.Derived) + } +} + +func TestLinkPackagesIsDeterministic(t *testing.T) { + build := func() *Graph { + return linkGraph( + pkgNode("pkgA", "com.depop.auth.client", "libraries"), + pkgNode("pkgB", "com.depop.data.store", "data"), + importNode("imp1", "com.depop.auth.client.AuthClient", "martech"), + importNode("imp2", "com.depop.data.store.Reader", "product"), + importNode("imp3", "com.depop.auth.client.Jwt", "ads"), + ) + } + + first := build() + LinkPackages(first) + want := derivedEdges(first) + if len(want) != 3 { + t.Fatalf("got %d derived edges, want 3", len(want)) + } + + for i := 0; i < 20; i++ { + g := build() + LinkPackages(g) + got := derivedEdges(g) + if strings.Join(got, "|") != strings.Join(want, "|") { + t.Fatalf("run %d produced %v, first run produced %v", i, got, want) + } + } +} + +// Derived edges must be distinguishable in the output, or a reader takes an +// inference on the same footing as a recorded fact. +func TestDerivedEdgesRenderDistinctly(t *testing.T) { + g := linkGraph( + pkgNode("pkg1", "com.depop.auth.client", "libraries"), + importNode("imp1", "com.depop.auth.client.AuthClient", "martech"), + ) + LinkPackages(g) + + sub, err := g.Subgraph(GraphOptions{}) + if err != nil { + t.Fatalf("Subgraph: %v", err) + } + + mermaid := RenderMermaid(sub) + if !strings.Contains(mermaid, "-.->|DEPENDS_ON|") { + t.Errorf("derived edge is not dashed in mermaid:\n%s", mermaid) + } + if !strings.Contains(mermaid, "1 of those are derived") { + t.Errorf("header does not count derived edges:\n%s", mermaid) + } + + dot := RenderDOT(sub) + if !strings.Contains(dot, `[label="DEPENDS_ON", style=dashed]`) { + t.Errorf("derived edge is not dashed in dot:\n%s", dot) + } +} diff --git a/src/kg/internal/knowledge/graph_render.go b/src/kg/internal/knowledge/graph_render.go index 2796c42..af35744 100644 --- a/src/kg/internal/knowledge/graph_render.go +++ b/src/kg/internal/knowledge/graph_render.go @@ -99,7 +99,11 @@ func RenderMermaid(sub Subgraph) string { } for _, e := range sub.Edges { - fmt.Fprintf(&b, " %s -->|%s| %s\n", ids[e.FromID], mermaidLabel(e.Type), ids[e.ToID]) + arrow := "-->" + if e.Derived { + arrow = "-.->" + } + fmt.Fprintf(&b, " %s %s|%s| %s\n", ids[e.FromID], arrow, mermaidLabel(e.Type), ids[e.ToID]) } if sub.RootID != "" { @@ -131,7 +135,11 @@ func RenderDOT(sub Subgraph) string { } for _, e := range sub.Edges { - fmt.Fprintf(&b, " %s -> %s [label=%s];\n", ids[e.FromID], ids[e.ToID], dotQuote(e.Type)) + attrs := "label=" + dotQuote(e.Type) + if e.Derived { + attrs += ", style=dashed" + } + fmt.Fprintf(&b, " %s -> %s [%s];\n", ids[e.FromID], ids[e.ToID], attrs) } b.WriteString("}\n") @@ -141,8 +149,20 @@ func RenderDOT(sub Subgraph) string { // writeHeader writes the provenance comment both formats carry: what is in the // picture, and — the part that matters — whether anything was left out of it. func writeHeader(b *strings.Builder, sub Subgraph, comment string) { + derived := 0 + for _, e := range sub.Edges { + if e.Derived { + derived++ + } + } fmt.Fprintf(b, "%s kg graph: %d node(s), %d relation(s) of %d entities in the project\n", comment, len(sub.Nodes), len(sub.Edges), sub.TotalNodes) + if derived > 0 { + // Said separately rather than folded into the count: these are edges + // this tool worked out, and a reader should not have to take them on + // the same footing as ones an indexer recorded. + fmt.Fprintf(b, "%s %d of those are derived cross-layer links, drawn dashed\n", comment, derived) + } if sub.Truncated { fmt.Fprintf(b, "%s TRUNCATED at the node limit — raise --limit to see the rest\n", comment) } From 45291ebe9838cdac0736c5cede49f4a31d2b33c1 Mon Sep 17 00:00:00 2001 From: Bryan Woodruff Date: Sat, 29 Aug 2026 07:46:38 -0700 Subject: [PATCH 4/6] test(kg): exercise the same-layer join guard with a type that still joins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rebasing onto #5 brought this test alongside #10's per-type join policy, which restricts joining to package and import — "this type's names are local to a repository; two matches are a coincidence, not an identity". The fixture used `type`, which no longer joins, so it reported 3 config nodes instead of 2: nothing fused, but only because nothing joined at all. The test would have kept passing while exercising none of the path it was written for. Switch the fixture to two packages named "config" in one layer plus a third in another — internal/api/config alongside internal/worker/config is an ordinary Go layout, so this is a more representative case than the original, not just a working one. Add a premise guard: assert JoinTypes actually covers package before drawing any conclusion from the count. Without it, a future narrowing of the policy silently turns this back into a test that passes for the wrong reason, which is exactly what the rebase just did. Both directions mutation-checked: dropping the same-layer guard fails with "config nodes = 1" (the fusion this test exists to catch), and narrowing DefaultJoinTypes to import alone fails the premise guard rather than passing vacuously. Co-Authored-By: Claude Opus 5 (1M context) --- .../knowledge/graph_federated_test.go | 35 ++++++++++++++----- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/src/kg/internal/knowledge/graph_federated_test.go b/src/kg/internal/knowledge/graph_federated_test.go index 6e49e49..6e6dff4 100644 --- a/src/kg/internal/knowledge/graph_federated_test.go +++ b/src/kg/internal/knowledge/graph_federated_test.go @@ -476,25 +476,42 @@ func TestLoadFederatedGraphReportsRemotesAsSkipped(t *testing.T) { // another layer, which is what makes the key joinable in the first place. func TestLoadFederatedGraphKeepsSameLayerNamesakesApart(t *testing.T) { aiDir := t.TempDir() - // Two unrelated Config types in one layer, plus a Config in another so the - // (name, type) key counts as joinable. + // Two unrelated packages named "config" in one layer — internal/api/config + // and internal/worker/config, which is an ordinary Go layout — plus a + // "config" package in another layer so the (name, type) key is joinable. + // The type must be one DefaultJoinTypes actually joins: only package and + // import cross layers, so a type or file namesake never reaches this path. seedLayer(t, aiDir, "payments", []string{"libs"}, fedEntity{name: "Root", entityType: EntityTypeFile}, - fedEntity{name: "Config", entityType: EntityTypeType}, - fedEntity{name: "Config", entityType: EntityTypeType}) + fedEntity{name: "config", entityType: EntityTypePackage}, + fedEntity{name: "config", entityType: EntityTypePackage}) seedLayer(t, aiDir, "libs", nil, - fedEntity{name: "Config", entityType: EntityTypeType}) + fedEntity{name: "config", entityType: EntityTypePackage}) - g, _ := loadFed(t, aiDir, "payments", 0) + g, report := loadFed(t, aiDir, "payments", 0) + + // Guard the premise: if the join policy stops covering package, this test + // would pass for the wrong reason — nothing joins, so nothing can fuse. + joined := false + for _, jt := range report.JoinTypes { + if strings.EqualFold(jt, EntityTypePackage) { + joined = true + } + } + if !joined { + t.Fatalf("JoinTypes = %v, which does not include %s — this fixture no longer exercises joining", + report.JoinTypes, EntityTypePackage) + } configs := 0 for _, n := range g.nodes { - if n.Name == "Config" { + if n.Name == "config" { configs++ } } - // payments' two stay separate; libs' joins onto one of them. + // payments' two stay separate; libs' joins onto one of them. 1 means the + // same-layer pair fused; 3 means nothing joined at all. if configs != 2 { - t.Errorf("Config nodes = %d, want 2 — the two same-layer Configs were fused into one", configs) + t.Errorf("config nodes = %d, want 2 — 1 means the two same-layer packages fused, 3 means no join happened", configs) } } From 73b25526469eb154f7cae55c9b2791b2d247aad5 Mon Sep 17 00:00:00 2001 From: Bryan Woodruff Date: Sat, 29 Aug 2026 07:59:07 -0700 Subject: [PATCH 5/6] fix(pr7): resolve an import that equals a package name; normalise JoinTypes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two of the three findings from #7's first review. The Critical is not addressed here — see below. resolvePackage tested only PROPER prefixes: the loop started at len(parts)-1, so it never compared the whole import name against a package name. The documented rule is "the longest package name that is a dotted prefix of it", and a string is a prefix of itself. That gap has a specific, ordinary victim. Java wildcard imports arrive as exactly the package name — extractImportPath keeps only the scoped_identifier and drops the trailing asterisk, so `import com.depop.auth.client.*;` becomes "com.depop.auth.client". Every one of those silently failed to resolve. Kotlin escaped it by accident: its extractor keeps the ".*" verbatim, leaving a longer string that the proper-prefix loop does reach. Reproduced before fixing (Derived = 0, want 1) and mutation-checked: restoring len(parts)-1 fails the new test with that same figure. Also normalised report.JoinTypes to lower case. Matching was already case-insensitive via lowerSet, but the raw input was stored, so `--join-types Package,IMPORT` printed back "Package, IMPORT". Not fixed: the Critical, that minPackageSegments = 3 filters out every package entity this repo's indexer can produce. Verified — EntityTypePackage is written in exactly one place, the Go package_clause handler, and a Go package name is a bare identifier with no dots. No Kotlin/Java/Scala package declaration is indexed at all. So LinkPackages returns Derived: 0 against anything this indexer produced, and the design doc's claim that "the indexers mint package entities only for dotted namespaces" is backwards for this codebase. That one needs a decision rather than a patch: either land JVM package indexing, or correct the doc and re-state where the estate's 2,525 edges actually came from. Both change what this PR claims to be, so they are the author's call. Co-Authored-By: Claude Opus 5 (1M context) --- src/kg/internal/knowledge/graph_federated.go | 8 ++++++- src/kg/internal/knowledge/graph_link.go | 10 ++++++++- src/kg/internal/knowledge/graph_link_test.go | 22 ++++++++++++++++++++ 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/src/kg/internal/knowledge/graph_federated.go b/src/kg/internal/knowledge/graph_federated.go index b8bed13..6ba839a 100644 --- a/src/kg/internal/knowledge/graph_federated.go +++ b/src/kg/internal/knowledge/graph_federated.go @@ -165,7 +165,13 @@ func LoadFederatedGraph(opts FederatedGraphOptions) (*Graph, *FederationReport, joinTypes = DefaultJoinTypes } eligible := lowerSet(joinTypes) - report.JoinTypes = append([]string{}, joinTypes...) + // Lower-cased for the report: matching is already case-insensitive via + // lowerSet, but storing the raw input meant `--join-types Package,IMPORT` + // printed back "Package, IMPORT" instead of a normalised form. + report.JoinTypes = make([]string, 0, len(joinTypes)) + for _, t := range joinTypes { + report.JoinTypes = append(report.JoinTypes, strings.ToLower(t)) + } joinable := make(map[ntKey]bool) for key, count := range layerCount { diff --git a/src/kg/internal/knowledge/graph_link.go b/src/kg/internal/knowledge/graph_link.go index aa1a686..b984060 100644 --- a/src/kg/internal/knowledge/graph_link.go +++ b/src/kg/internal/knowledge/graph_link.go @@ -156,9 +156,17 @@ func packageIndex(g *Graph) map[string]*packageTarget { // resolvePackage finds the longest package name that prefixes importName, // returning the name and its target. Longest-first means // com.depop.auth.client beats com.depop.auth, which is the specific answer. +// +// The loop starts at len(parts), not len(parts)-1: a string is a prefix of +// itself, and an import equal to a package name is the ordinary shape of a +// Java wildcard import. extractImportPath keeps only the scoped_identifier and +// drops the trailing asterisk, so `import com.depop.auth.client.*;` arrives as +// "com.depop.auth.client" — exactly the package name. Testing only proper +// prefixes dropped every one of those silently. (Kotlin escaped it by accident: +// its extractor keeps the ".*" verbatim, leaving a longer string.) func resolvePackage(importName string, targets map[string]*packageTarget) (string, *packageTarget) { parts := strings.Split(importName, packageSeparator) - for cut := len(parts) - 1; cut >= minPackageSegments; cut-- { + for cut := len(parts); cut >= minPackageSegments; cut-- { candidate := strings.Join(parts[:cut], packageSeparator) if target, ok := targets[candidate]; ok { return candidate, target diff --git a/src/kg/internal/knowledge/graph_link_test.go b/src/kg/internal/knowledge/graph_link_test.go index c367a3f..5a3539d 100644 --- a/src/kg/internal/knowledge/graph_link_test.go +++ b/src/kg/internal/knowledge/graph_link_test.go @@ -238,3 +238,25 @@ func TestDerivedEdgesRenderDistinctly(t *testing.T) { t.Errorf("derived edge is not dashed in dot:\n%s", dot) } } + +// A Java wildcard import resolves to the package name itself. extractImportPath +// keeps only the scoped_identifier and drops the trailing asterisk, so +// `import com.depop.auth.client.*;` arrives as "com.depop.auth.client" — +// identical to the package it names. +// +// The documented rule is "the longest package name that is a dotted prefix of +// it", and a string is a prefix of itself, so this must resolve. Kotlin happens +// to escape the bug only because its extractor keeps the ".*" suffix verbatim, +// leaving a longer string that the proper-prefix loop does reach. +func TestLinkPackagesResolvesAnImportEqualToThePackageName(t *testing.T) { + g := linkGraph( + pkgNode("pkg1", "com.depop.auth.client", "libraries"), + importNode("imp1", "com.depop.auth.client", "martech"), + ) + + report := LinkPackages(g) + + if report.Derived != 1 { + t.Errorf("Derived = %d, want 1 — an import equal to the package name did not resolve", report.Derived) + } +} From 03200e7690c76cd97612c4a4627cd1d999d3fb21 Mon Sep 17 00:00:00 2001 From: Bryan Woodruff Date: Sat, 29 Aug 2026 15:01:51 -0700 Subject: [PATCH 6/6] fix(pr7): prove cross-layer linking on indexed source; correct the design doc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reviewer's Critical was that this feature could not derive anything from real data: minPackageSegments filtered out every package entity the indexer produced, because Go's bare identifier was the only source and never has a dot. That was accurate. JVM package indexing has since landed separately, so the dependency now exists and the claim can be demonstrated rather than argued. Add an end-to-end test that indexes actual Java source into two scope databases — a library layer declaring com.depop.auth.client, a consumer layer importing from it — federates them, and asserts a derived cross-layer edge. Nothing is hand-built: the graph is what `kg index` produces. Mutation-checked against the pre-indexing world: removing package_declaration from the Java config fails this test with "Derived = 0", which is exactly the symptom the review described. That makes the test a regression guard on the dependency, not just on this code. Correct the design doc's Follow-up, which stated the inverse of reality — that the indexers mint package entities "only for dotted namespaces", when Go's undotted name was the only kind minted. Replaced with a table of what each language actually yields and why Go cannot link, and the remaining npm/go.mod gap restated on that footing. Flag the acceptance figures rather than delete them. The 2,525 edges cannot have come from `kg index` as it shipped when they were recorded, for the reason above, so they are marked as predating JVM package indexing and needing a re-run against a freshly indexed estate. Re-deriving them needs that estate, so it is not something this change can settle. Co-Authored-By: Claude Opus 5 (1M context) --- docs/kg-graph-linking-design.md | 31 ++++++- .../knowledge/graph_link_integration_test.go | 92 +++++++++++++++++++ 2 files changed, 118 insertions(+), 5 deletions(-) create mode 100644 src/kg/internal/knowledge/graph_link_integration_test.go diff --git a/docs/kg-graph-linking-design.md b/docs/kg-graph-linking-design.md index 8230101..78e7424 100644 --- a/docs/kg-graph-linking-design.md +++ b/docs/kg-graph-linking-design.md @@ -206,6 +206,12 @@ and the count of name-coincidence edges is zero under default settings. Yield on the estate, as implemented: **2,525 derived `DEPENDS_ON` edges**, replacing 67,263 manufactured ones. +> **These figures predate JVM package indexing and have not been reproduced +> since.** They cannot have come from `kg index` as it shipped at the time — +> the only package entities it minted were Go's undotted names, which this +> rule filters out. Re-run the measurement against a freshly indexed estate +> before treating the numbers as current. + The gate predicted 845, and both numbers are right: 845 is the count of distinct import *names* that resolve, while 2,525 counts import *nodes* — the same name appears as a separate node in each layer that imports it, since imports join only @@ -215,11 +221,26 @@ exactly. ## Follow-up -**Package entities for npm and Go.** The measurement shows the indexers mint -`package` entities only for dotted namespaces. Reading `package.json` `name` fields -and `go.mod` module paths would extend cross-layer linking to the web, client and -tooling layers, which today get no derived edges at all. That is an indexer change, -independent of this proposal and probably larger than it. +**Package entities for npm and Go.** What the indexers mint is now: + +| language | package entity | linkable | +|---|---|---| +| Java, Kotlin, Scala | dotted namespace (`com.depop.auth.client`) | yes | +| Go | bare identifier (`auth`) | no — one segment, below the specificity floor | +| TypeScript, JavaScript, Python, C/C++ | none | no | + +An earlier draft of this section had that backwards, saying the indexers mint +package entities "only for dotted namespaces". The reverse was true when it was +written: Go's `package_clause` was the *only* source, and a Go package name never +contains a dot, so every package entity was filtered out and this proposal +derived nothing from indexed data. JVM package declarations were not indexed at +all until they were added separately (see the package-indexing change that +precedes this one), which is what makes the rule above work. + +The remaining gap is npm and Go. Reading `package.json` `name` fields and +`go.mod` module paths would give the web, client and tooling layers a namespace +specific enough to link, where today they get no derived edges. That is an +indexer change, independent of this proposal and probably larger than it. Once edges are real, aggregation becomes worth building — `--rollup layer|package`, collapsing the graph to a granularity a person can read (tens of nodes, weighted diff --git a/src/kg/internal/knowledge/graph_link_integration_test.go b/src/kg/internal/knowledge/graph_link_integration_test.go new file mode 100644 index 0000000..22629c8 --- /dev/null +++ b/src/kg/internal/knowledge/graph_link_integration_test.go @@ -0,0 +1,92 @@ +package knowledge + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" +) + +// indexLayerFromSource writes a scope config and indexes real source into that +// scope's database, so the resulting graph is what `kg index` would actually +// produce rather than hand-built nodes. +func indexLayerFromSource(t *testing.T, aiDir, name string, layers []string, files map[string]string) { + t.Helper() + + scopeDir := filepath.Join(aiDir, "scope") + if err := os.MkdirAll(scopeDir, 0o755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + cfg := ScopeConfig{Name: name, Database: name + ".db", Layers: layers} + data, err := json.Marshal(cfg) + if err != nil { + t.Fatalf("marshal scope %s: %v", name, err) + } + if err := os.WriteFile(filepath.Join(scopeDir, name+".json"), data, 0o644); err != nil { + t.Fatalf("write scope %s: %v", name, err) + } + + srcDir := filepath.Join(t.TempDir(), name) + if err := os.MkdirAll(srcDir, 0o755); err != nil { + t.Fatalf("MkdirAll src: %v", err) + } + for file, content := range files { + if err := os.WriteFile(filepath.Join(srcDir, file), []byte(content), 0o644); err != nil { + t.Fatalf("write %s: %v", file, err) + } + } + + store, err := OpenStore(filepath.Join(aiDir, cfg.Database)) + if err != nil { + t.Fatalf("OpenStore %s: %v", name, err) + } + defer store.Close() + + idx, err := NewIndexer(store, fedProject, srcDir) + if err != nil { + t.Fatalf("NewIndexer %s: %v", name, err) + } + if _, err := idx.Index(); err != nil { + t.Fatalf("Index %s: %v", name, err) + } +} + +// The end-to-end claim: cross-layer package linking derives real edges from +// what the indexer actually produces, not only from hand-built fixtures. +// +// This could not pass before JVM package declarations were indexed — the only +// package entities were Go's bare identifiers, which never reach +// minPackageSegments, so packageIndex filtered every one of them out and +// LinkPackages returned Derived: 0 against any indexed corpus. +func TestLinkPackagesDerivesFromIndexedJVMSource(t *testing.T) { + if testing.Short() { + t.Skip("Skipping integration test in short mode") + } + + aiDir := t.TempDir() + + // The library layer declares the package. + indexLayerFromSource(t, aiDir, "libraries", nil, map[string]string{ + "AuthClient.java": "package com.depop.auth.client;\n\npublic class AuthClient {}\n", + }) + // The consumer layer imports a symbol from it. + indexLayerFromSource(t, aiDir, "martech", []string{"libraries"}, map[string]string{ + "Checkout.java": "package com.depop.martech.checkout;\n\nimport com.depop.auth.client.AuthClient;\n\npublic class Checkout {}\n", + }) + + scope, err := LoadScopeConfig(aiDir, "martech") + if err != nil { + t.Fatalf("LoadScopeConfig: %v", err) + } + _, report, err := LoadFederatedGraph(FederatedGraphOptions{ + AIDir: aiDir, Scope: scope, ProjectID: fedProject, + }) + if err != nil { + t.Fatalf("LoadFederatedGraph: %v", err) + } + + if report.Link.Derived < 1 { + t.Errorf("Derived = %d, want at least 1 — no cross-layer edge was derived from indexed source", + report.Link.Derived) + } +}