feat(kg): kg graph --federated — one graph across a scope and all its layers - #6
Merged
Merged
Conversation
kg's federation is search-only by design — SearchLayer merges query results and nothing enumerates entities across databases — so a picture of a federated estate has to be assembled here. That raises a question searching never has to answer: when are two rows in two databases the same thing? They are joined on (name, type), guarded. Unguarded, the join is worse than no join at all. Measured on a real 59-layer estate, the most widely shared names are markdown headings from a docs template (Service Overview, Deployment, Known Issues and Failure Modes) and Helm values keys indexed as types (accountName, environment, chart) — several in all 60 layers. Joining those fuses every unrelated service into one hub. A name in more than MaxJoinLayers layers is therefore read as boilerplate and left alone, and the report says which, so the threshold can be argued with rather than trusted. Two layers can also mint the same ID for different things: indexer IDs are repo-relative paths, so "import:.." means something different in each layer. Those are renamed rather than merged — reusing the ID fused exactly the nodes the guard had just decided to keep apart, and the node counts stopped reconciling, which is how it was caught. Databases are read one at a time and released, so peak memory is the merged graph plus the largest layer, not all of them. 61 layers, 668k entities, 1.2M relations: ~6s and 1.2GB resident. Renderers group nodes into a mermaid subgraph per layer — sixty databases in one flat node list is not a picture anyone can read.
Renders a scope together with every layer it federates with. --layer narrows the load, --join-max-layers argues with the boilerplate guard. What the load suppressed, renamed, or could not open goes to stderr rather than being left to be inferred from a graph that looks complete. A layer that fails to open is a warning, not a fatal error: a render missing one database is still useful, a silent hole is not. --layer and --join-max-layers without --federated are an error. Ignoring them would render a single database and look like it had done what was asked.
Includes the measured numbers and the real suppression output, because the join guard is only defensible if the reader can see what it excluded.
Contributor
Author
|
Measured this branch's output against the real 61-layer estate before building the next slice, and the result bears on the defaults here. All 67,263 cross-layer relations this join produces are manufactured by it. No indexer writes a relation across databases, so the federated union is genuinely 61 disconnected components — 100% of the bridges come from The guard catches the widespread case and misses the local-name case. The distinction that matters is per entity type, not per count: package names are chosen to be globally meaningful, function names are not. Proposed fix in #7 (spec only): per-type join policy — Everything else on this branch stands: the merge, the ID-collision fix, per-layer subgraph rendering, the report. But I'd either change the defaults here before merge, or add a line to the docs saying plainly that cross-layer edges are name-derived and must not be read as dependencies. Happy to do either — say which. |
bawoodruff
added a commit
that referenced
this pull request
Aug 28, 2026
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.
bawoodruff
added a commit
that referenced
this pull request
Aug 29, 2026
… layers (#6) * feat(kg): federated graph loading across a scope and its layers kg's federation is search-only by design — SearchLayer merges query results and nothing enumerates entities across databases — so a picture of a federated estate has to be assembled here. That raises a question searching never has to answer: when are two rows in two databases the same thing? They are joined on (name, type), guarded. Unguarded, the join is worse than no join at all. Measured on a real 59-layer estate, the most widely shared names are markdown headings from a docs template (Service Overview, Deployment, Known Issues and Failure Modes) and Helm values keys indexed as types (accountName, environment, chart) — several in all 60 layers. Joining those fuses every unrelated service into one hub. A name in more than MaxJoinLayers layers is therefore read as boilerplate and left alone, and the report says which, so the threshold can be argued with rather than trusted. Two layers can also mint the same ID for different things: indexer IDs are repo-relative paths, so "import:.." means something different in each layer. Those are renamed rather than merged — reusing the ID fused exactly the nodes the guard had just decided to keep apart, and the node counts stopped reconciling, which is how it was caught. Databases are read one at a time and released, so peak memory is the merged graph plus the largest layer, not all of them. 61 layers, 668k entities, 1.2M relations: ~6s and 1.2GB resident. Renderers group nodes into a mermaid subgraph per layer — sixty databases in one flat node list is not a picture anyone can read. * feat(kg): kg graph --federated Renders a scope together with every layer it federates with. --layer narrows the load, --join-max-layers argues with the boilerplate guard. What the load suppressed, renamed, or could not open goes to stderr rather than being left to be inferred from a graph that looks complete. A layer that fails to open is a warning, not a fatal error: a render missing one database is still useful, a silent hole is not. --layer and --join-max-layers without --federated are an error. Ignoring them would render a single database and look like it had done what was asked. * docs(kg): document federated rendering Includes the measured numbers and the real suppression output, because the join guard is only defensible if the reader can see what it excluded.
bawoodruff
added a commit
that referenced
this pull request
Aug 29, 2026
…SON (#5) * feat(kg): subgraph traversal and mermaid/DOT/JSON renderers The data layer behind `kg graph`. LoadGraph reads a project's entities and relations in one pass; everything after that is a pure function over an adjacency list, which is why depth, direction, filtering and truncation can be tested without opening a database. Output ordering is deterministic by construction, not by accident: map iteration decides nothing. That matters most under --limit, where the order is what picks which nodes survive — an unstable one would draw a different picture every run. Renderers number nodes positionally (n0, n1, …) rather than reusing entity IDs, which are UUIDs or strings like "function:src/a.go:F" that neither mermaid nor DOT accepts unquoted. Only label text is left to escape. No CLI yet; that lands next. * feat(kg): kg graph command Renders a slice of the graph to stdout or a file. --root plus --depth is the path this is built around; the whole-graph render is the fallback, because at this repo's ~1200 entities every renderer turns the full graph into a hairball. --root takes a name as well as an ID: the IDs kg mints are UUIDs or strings like "function:src/a.go:F", and nobody types those. An ambiguous name lists the candidates rather than picking one — `kg graph --root main` on a Go project means two different entities and guessing would be worse than asking. Validation happens before the graph loads, so a rejected invocation cannot leave a half-written document behind, and notes about truncation go to stderr so a piped render stays a clean document. * docs(kg): document kg graph Adds the command reference, and corrects a stale line in src/kg/README.md while passing through it: it listed export, graph, gc and embed as unimplemented stubs that "print Not yet implemented and exit 0". Export and embed have been real for some time, gc does not exist under any name, and graph is real as of this branch — so every claim in that sentence was wrong. * feat(kg): kg graph --federated — one graph across a scope and all its layers (#6) * feat(kg): federated graph loading across a scope and its layers kg's federation is search-only by design — SearchLayer merges query results and nothing enumerates entities across databases — so a picture of a federated estate has to be assembled here. That raises a question searching never has to answer: when are two rows in two databases the same thing? They are joined on (name, type), guarded. Unguarded, the join is worse than no join at all. Measured on a real 59-layer estate, the most widely shared names are markdown headings from a docs template (Service Overview, Deployment, Known Issues and Failure Modes) and Helm values keys indexed as types (accountName, environment, chart) — several in all 60 layers. Joining those fuses every unrelated service into one hub. A name in more than MaxJoinLayers layers is therefore read as boilerplate and left alone, and the report says which, so the threshold can be argued with rather than trusted. Two layers can also mint the same ID for different things: indexer IDs are repo-relative paths, so "import:.." means something different in each layer. Those are renamed rather than merged — reusing the ID fused exactly the nodes the guard had just decided to keep apart, and the node counts stopped reconciling, which is how it was caught. Databases are read one at a time and released, so peak memory is the merged graph plus the largest layer, not all of them. 61 layers, 668k entities, 1.2M relations: ~6s and 1.2GB resident. Renderers group nodes into a mermaid subgraph per layer — sixty databases in one flat node list is not a picture anyone can read. * feat(kg): kg graph --federated Renders a scope together with every layer it federates with. --layer narrows the load, --join-max-layers argues with the boilerplate guard. What the load suppressed, renamed, or could not open goes to stderr rather than being left to be inferred from a graph that looks complete. A layer that fails to open is a warning, not a fatal error: a render missing one database is still useful, a silent hole is not. --layer and --join-max-layers without --federated are an error. Ignoring them would render a single database and look like it had done what was asked. * docs(kg): document federated rendering Includes the measured numbers and the real suppression output, because the join guard is only defensible if the reader can see what it excluded. * fix(kg): --type filters the drawing, not the walk Found by the automated reviewer on #5, verified against the code. `--type` was pruning traversal, not just output. In walk, a neighbour only entered the next frontier when admit() returned true, and admit() rejects any non-root node failing the type filter — so an excluded node became a dead end and nothing behind it was reachable, however well it matched. app.go -CONTAINS-> foo (function) -IMPORTS-> fmt (import) With --type import --depth 2, fmt is a match two hops out, but foo is filtered, so the walk stopped at foo and fmt never appeared. Nothing signalled the loss: unlike --limit there is no Truncated flag for it, so the render simply came back quietly incomplete. That contradicted the documented contract, which says --rel "constrains what the walk follows, not just what is drawn" and that --type "works the other way". The code made --type constrain reachability exactly like --rel, just keyed on node type. Also worse than it first looks: visited[neighbour] is set *before* admit(), so a type-rejected node was marked visited too, and could not be reached later by another path either. Separate the two decisions. admit() still governs membership; the walk now continues through a node regardless of the type filter, and only the node budget stops the expansion — the one condition where continuing is pointless, since nothing further can be admitted. Two smaller fixes from the same review: - `kg graph -o FILE` discarded the file's Close error, so a failure that only surfaces on close (full disk, late NFS error) printed "Wrote N node(s)" and exited 0 over a truncated file. RunE takes a named return and the deferred close now reports. - federationOrder did not dedupe, so a name repeated in Layers (or a layer sharing the scope's own name) loaded that database twice and then ran the ID-collision rename path against its own nodes. The surviving copy keeps the higher-priority slot. Every new test was mutation-checked: restoring the frontier gating fails both traversal cases, and removing the dedupe fails the order case. The docs gain a sentence making the pass-through contract explicit so it cannot drift back. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(kg): never destroy an -o file before the render succeeds Second review round on #5 found this, and it is worse than a validation ordering slip. RunE opened the output with os.Create up front, which is O_TRUNC, so the file was emptied the moment the command started — before settings.validate() (which runs later, inside runGraph/runFederatedGraph), before --root resolution, and before openTarget even opened the store. Any of those failing left the target already truncated. The documented workflow makes that costly: the CLI reference recommends committing a diagram and refreshing it with the same command. Rename the root entity, rerun, and the command fails with "no entity with ID or name ..." — after the committed diagram has been reduced to an empty file. It also contradicted the code's own stated intent: the comment on graphSettings.validate says a bad invocation "fails before any database is opened", which only held when runGraph was called directly, as the tests did. Nothing exercised the real command path with -o set, so the ordering was completely uncovered. Render into a buffer and touch the target only after the whole render succeeds. Writing goes through writeFileAtomic — a temporary file in the same directory, then a rename — so a failure part-way through the write leaves the previous contents rather than a half-written diagram; the file is either the old one or a complete new one. It also restores 0644, since CreateTemp makes 0600, and reports the temp file's Close error rather than renaming a truncated file over a good one. That subsumes the earlier deferred-Close fix, so the named return goes away again. Also short-circuit selectAll once the node budget is spent, matching walk, which the same review noted as an inconsistency. Tests, both mutation-checked: restoring os.Create-up-front fails the new regression test with exactly the reported symptom (output file = ""), and TestWriteFileAtomic covers replacement, mode, and leaving no temp files behind. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(pr5): report skipped remote layers, validate before opening the store Third review round on #5. Both earlier findings were confirmed fixed; these are the two it raised against the current head. Major — `--federated` silently omitted a scope's remote hub layers. federationOrder only walks Scope.Layers, and nothing in the graph path ever reads Scope.Remotes, while search does federate them (search.go, federated.go, mcp_server.go). So a scope configured with remotes rendered a quietly smaller graph than the flag's own help promised: "every layer it federates with". Rendering them is not a small change and is arguably not wanted here: loading a layer's rows needs raw Cypher against a local *Store, and a hub layer is reached over HTTP. So make the limitation visible rather than implementing remote graph loading in a PR about local rendering. Each remote is reported exactly like a local layer that could not be opened, so printFederationReport already warns about it with no new plumbing, and the help text and docs now say local layers instead of implying parity with search. Minor — the non-federated path opened the store before validating. The federated path validated first, so `kg graph --format bogus` opened a Kuzu database before failing on one branch and not the other, contradicting graphSettings.validate's own docstring ("a bad invocation fails before any database is opened"). Hoisted the call into RunE so it holds for the command, not just for the functions the tests call directly. The reviewer also noted the earlier output-file test masked this ordering, since it runs where no project exists and the store open fails regardless. The new test discriminates on which error comes back: the format error means validation ran first, a store error means it did not — and the mutation check confirms it, failing with exactly that store-open error when the hoist is removed. Both new tests mutation-checked: dropping the remotes report fails with "remotes were dropped silently", and un-hoisting validate fails with the database-open error. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(pr5): never fuse two same-layer entities that share a name Fourth review round on #5 found a Critical in the federated join itself. The join is documented as a cross-database rule — "two rows in two databases are the same node when their (name, type) match" — but the implementation only asked whether the key had been claimed, never by which layer. Iterating a single layer's nodes, the first row to claim a joinable (name, type) became canonical and the next row with that same pair was merged into it, even though both came from the same database and were unrelated entities. Nothing dedupes (name, type) inside a database: CreateEntity issues a bare CREATE with a fresh UUID per row, so a Go codebase holding internal/api/Config and internal/worker/Config alongside each other is ordinary. Such a pair fused whenever the same name also appeared in another layer, which is exactly the condition that made the key joinable in the first place. The failure was silent in every channel. appendLayer dedupes the layer name, so the surviving node still listed one layer and looked like an ordinary un-joined node; the merged-away row simply skipped load.Nodes++, so TestLoadFederatedGraphNodeCountsReconcile still balanced while a real entity had disappeared and its edges had been redirected onto an unrelated node. Record the owning layer alongside the claimed ID and merge only when the arriving row comes from a different layer. A same-layer namesake falls through to the normal add path, keeping its own identity, and must not steal the claim — otherwise the guard would compare against the wrong owner. Reproduced before fixing: two Configs in one layer plus a third in another collapsed to a single node (Config nodes = 1, want 2), and the mutation check restores exactly that failure. Also fixed the reported minor: printFederationReport counted every entry in report.Layers, including failed locals and the newly-added remote entries, so a scope with 2 local layers and 3 remotes announced "Federated 6 layer(s)" beside the node and edge totals of 2. It now counts contributing layers, leaving the warning lines to name what was skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
bawoodruff
added a commit
that referenced
this pull request
Aug 29, 2026
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.
bawoodruff
added a commit
that referenced
this pull request
Aug 29, 2026
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.
bawoodruff
added a commit
that referenced
this pull request
Aug 29, 2026
* docs(kg): design proposal for cross-layer entity linking 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): measurement gate results — prefix matching, not exact 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. * feat(kg): per-type join policy and derived cross-layer package links (#10) * 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. * test(kg): exercise the same-layer join guard with a type that still joins 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) <noreply@anthropic.com> * fix(pr7): resolve an import that equals a package name; normalise JoinTypes 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) <noreply@anthropic.com> * fix(pr7): prove cross-layer linking on indexed source; correct the design doc 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) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
bawoodruff
added a commit
that referenced
this pull request
Aug 30, 2026
The estate was re-indexed with 3ac2f1a (971 s, 61 scopes, no failures, all 420 observations preserved) and every figure re-derived. Re-indexing lowers the derived-edge count rather than raising it: 2,532 -> 2,240, while same-layer resolutions more than double. Both movements are the rule behaving correctly on better data. com.depop.common was indexed only in libraries, so every clients import of it looked like a cross-repo dependency; now that clients declares it too, the name resolves to two layers and the one-layer rule abstains. Elsewhere longest-prefix now finds a specific local package where it previously settled for a shorter one defined elsewhere. Edges drawn because a repository's own copy of a package had not been indexed were never dependencies, so fewer of them is a more truthful graph — and it sharpens the open question about ambiguity, which grows as coverage improves. Also corrects a miscount of my own: "seventy-five .java files and no Kotlin" came from find -maxdepth 4, which misses trees nested deeper, the Android app among them. The estate holds 30,906 .scala, 7,591 .kt and 1,835 .java files, so 5211bc2 gave package declarations to about nine thousand files. CHANGELOG: --federated was #6 not #5, linking was #10 not #7, and its package-indexing entry repeated the same wrong Scala claim this branch corrects. The kg graph flag table was missing --scope and --personal, both of which its own examples use.
bawoodruff
added a commit
that referenced
this pull request
Aug 30, 2026
…ion (#16) * docs(kg): re-measure the estate figures, and withdraw a wrong correction The pending-re-measurement warning said the derived-edge figures could not have come from indexed data, because Go's package_clause was supposedly the only source of package entities before 5211bc2 and Go names carry no dots. Two facts say otherwise: package_clause is Scala's tree-sitter node as well as Go's, and indexer_treesitter.go matched it generically at 5211bc2^; and the estate's databases, indexed by v0.1.0-34, hold 4,385 package entities with three or more dotted segments, one of which resolves to .scala files. 5211bc2 added Java and Kotlin, not Scala. So the figures were sound. Re-derived at 3ac2f1a they are 2,532 derived edges (was 2,525), 3,355 ambiguous (was 3,359), 5,046 same-layer (was 5,031). The +7 is the wildcard-import fix: a Java `import com.x.y.*;` reaches the resolver as a string identical to the package name, and the original loop tested only proper prefixes, so those were dropped silently. Also records what the warning's remedy would actually buy. Package entities are minted at index time, so a newer binary over older databases changes nothing; re-indexing is what is required. By file count that is 75 .java files and no Kotlin against 12,537 already-indexed .scala — the unindexed mass is Python, TypeScript and Go, which 5211bc2 did not touch. * docs(kg): re-measure on a re-indexed estate; correct two miscounts The estate was re-indexed with 3ac2f1a (971 s, 61 scopes, no failures, all 420 observations preserved) and every figure re-derived. Re-indexing lowers the derived-edge count rather than raising it: 2,532 -> 2,240, while same-layer resolutions more than double. Both movements are the rule behaving correctly on better data. com.depop.common was indexed only in libraries, so every clients import of it looked like a cross-repo dependency; now that clients declares it too, the name resolves to two layers and the one-layer rule abstains. Elsewhere longest-prefix now finds a specific local package where it previously settled for a shorter one defined elsewhere. Edges drawn because a repository's own copy of a package had not been indexed were never dependencies, so fewer of them is a more truthful graph — and it sharpens the open question about ambiguity, which grows as coverage improves. Also corrects a miscount of my own: "seventy-five .java files and no Kotlin" came from find -maxdepth 4, which misses trees nested deeper, the Android app among them. The estate holds 30,906 .scala, 7,591 .kt and 1,835 .java files, so 5211bc2 gave package declarations to about nine thousand files. CHANGELOG: --federated was #6 not #5, linking was #10 not #7, and its package-indexing entry repeated the same wrong Scala claim this branch corrects. The kg graph flag table was missing --scope and --personal, both of which its own examples use. * docs(kg): bring the open questions in line with the re-measurement Question 3 still quoted the pre-re-index split (3,204 against 845) while the section above it reported 2,240 kept against 3,346 discarded, which read as the document contradicting itself. It also understated the problem. The discarded share grows as indexing coverage improves — a package indexed in more repositories resolves to more layers — so the linking weakens over time rather than strengthening. Records the option the re-measurement suggests: preferring the importing layer's own definition, which would have made com.depop.common same-layer and drawn nothing, instead of discarding it as ambiguous.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #5 — base is
feat/kg-graph-export, notmain. Review that one first; this rebases ontomainonce it merges.What
kg graph --federatedrenders a scope together with every layer it federates with, as one graph.Why it needs new code rather than a flag
kg's federation is search-only by design —
SearchLayer(kglib/federated.go:11) is exactlyHybridSearch+Close, and nothing enumerates entities across databases. Relations are stored per-database too, so a plain union of 61 layers is 61 disconnected components. What connects them is joining identities across layers, and that raises a question searching never has to answer: when are two rows in two databases the same thing?The join, and why it needed a guard
Joined on
(name, type). Unguarded, that join is worse than no join — measured against the real 59-layer estate before writing the merge:Those are Helm values keys indexed as types. Just behind them:
Service Overview,Deployment,Known Issues and Failure Modes— markdown headings from a documentation template, present in every service's docs. Join on those and every unrelated service fuses into one hub through a node calledname, and the picture stops meaning anything.So a
(name, type)in more than--join-max-layerslayers (default 3) is read as boilerplate and left unjoined, and the command prints what it suppressed. The threshold is arguable rather than magic;--join-max-layers 60joins everything if you want to see it.On the full estate that leaves 18,300 identities joined (53,732 duplicate rows merged) and 5,112 suppressed.
What it draws, on four layers:
Six separate
AddressClientdefinitions across three layers. Worth being precise in review: the join means same name and type, which surfaces genuinely shared symbols and independently duplicated implementations. The second is often the more interesting answer, but it is not "one entity used from many places" and the docs say so.A fusion bug the numbers caught
The report claimed 355,429 nodes; the render said 355,351. The gap was real: two layers minting the same ID — indexer IDs are repo-relative paths, so
import:..exists in every layer with a relative import — silently overwrote each other in the merged map. That fused nodes the guard had just decided to keep apart, and dropped one side's provenance. Colliding IDs are now renamed (<layer>::<id>) and counted in the report.TestLoadFederatedGraphRenamesCollidingIDscovers it, seeding a chosen ID via Cypher sinceCreateEntityonly mints UUIDs.Cost
Databases are read one at a time and released, so peak memory is the merged graph plus the largest single layer, not all 61. That costs a second pass — the first reads only
(name, type), which the guard needs before merging can start.Full estate — 61 layers, 667,858 entities, 1,223,605 relations: 6.4s, 1.2 GB resident.
--layernarrows it.Notes for review
--layer/--join-max-layerswithout--federatedare an error rather than ignored; ignoring them would render one database and look like it had honoured the request.--personal+--federatedis rejected — the personal store has no layers.Testing
make testgreen across kglib, kg, markitdown.~/Projects— every number quoted above is measured, not estimated.