From dcd79ad3c5ed5fff2e4776d6270b3d7d65745b6f Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Tue, 29 Sep 2026 17:03:34 +0000 Subject: [PATCH] fix(ui-map): count depth in folders that fork, so a Maven project opens on its packages (#2102) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Maven / Gradle project keeps every line of Java under src/main/java/org///, and those folders hold nothing but the next one. The Map cut one folder per level, so petclinic drew the whole program as one `src/main/java/org` box, and the deepest grouping option (4) stopped at `src/main/java/org/springframework` — no setting reached the packages seven levels down. A folder with exactly one subfolder and no file of its own (read from every indexed file, tests included, so ids don't move when tests are toggled) now joins the level below it. moduleIdFor takes the set as an optional fourth argument — without it nothing changes — and pickDefaultDepth counts the same levels. Collapsing never splits a group at the same depth (a pass-through folder has one child), so repos without such chains keep their grouping; a lone chain like koel's `app/Console` → `app/Console/Commands` is renamed to the folder that holds the files. The box label (the wire `label`, which nothing read) now elides a chain of three or more folders — `src/main/java/…/petclinic/owner` — and ModuleNode and the width calculation use it; the tooltip and side panel keep the full path. Everywhere else the label equals the id, so other maps look the same. petclinic's default map: 8 boxes (owner, vet, model, system, the package's root files, resources) instead of one; realworld opens on io/spring's api, application, core, graphql, infrastructure. Co-authored-by: Claude Opus 5.5 --- __tests__/ui-map-api.test.ts | 82 +++++++++++++-- docs/viewer-launch-changelog.md | 2 + src/ui-server/api/map.ts | 130 ++++++++++++++++++++---- ui/src/components/map/ModuleNode.svelte | 3 +- ui/src/lib/map-model.ts | 2 +- ui/src/lib/wire.ts | 1 + 6 files changed, 189 insertions(+), 31 deletions(-) diff --git a/__tests__/ui-map-api.test.ts b/__tests__/ui-map-api.test.ts index 6ba6d292..75e08316 100644 --- a/__tests__/ui-map-api.test.ts +++ b/__tests__/ui-map-api.test.ts @@ -28,6 +28,7 @@ import { moduleIdFor, normalizeRoot, pickDefaultDepth, + passThroughDirs, pickDefaultRoot, resetMapCache, } from '../src/ui-server/api/map'; @@ -227,20 +228,20 @@ afterAll(async () => { describe('moduleIdFor', () => { it('names a module after the first `depth` segments under the root', () => { - expect(moduleIdFor('src/core/engine.ts', 'src', 1)).toEqual({ id: 'src/core', facade: false }); - expect(moduleIdFor('src/a/b/c.ts', 'src', 2)).toEqual({ id: 'src/a/b', facade: false }); - expect(moduleIdFor('a/b/c.ts', '', 1)).toEqual({ id: 'a', facade: false }); + expect(moduleIdFor('src/core/engine.ts', 'src', 1)).toEqual({ id: 'src/core', label: 'src/core', facade: false }); + expect(moduleIdFor('src/a/b/c.ts', 'src', 2)).toEqual({ id: 'src/a/b', label: 'src/a/b', facade: false }); + expect(moduleIdFor('a/b/c.ts', '', 1)).toEqual({ id: 'a', label: 'a', facade: false }); }); it('keeps a façade as its own box and buckets the other loose files', () => { - expect(moduleIdFor('src/index.ts', 'src', 1)).toEqual({ id: 'src/index.ts', facade: true }); + expect(moduleIdFor('src/index.ts', 'src', 1)).toEqual({ id: 'src/index.ts', label: 'src/index.ts', facade: true }); expect(moduleIdFor('src/lib.rs', 'src', 1)?.facade).toBe(true); expect(moduleIdFor('pkg/__init__.py', 'pkg', 1)?.facade).toBe(true); expect(moduleIdFor('src/types.ts', 'src', 1)).toEqual({ id: 'src/(root files)', - facade: false, + label: 'src/(root files)', facade: false, }); - expect(moduleIdFor('types.ts', '', 1)).toEqual({ id: '(root files)', facade: false }); + expect(moduleIdFor('types.ts', '', 1)).toEqual({ id: '(root files)', label: '(root files)', facade: false }); }); it('buckets a loose file into the directory it is actually in, not the top one', () => { @@ -249,7 +250,7 @@ describe('moduleIdFor', () => { // file lives somewhere it does not. expect(moduleIdFor('src/a/loose.ts', 'src', 2)).toEqual({ id: 'src/a/(root files)', - facade: false, + label: 'src/a/(root files)', facade: false, }); }); @@ -258,6 +259,73 @@ describe('moduleIdFor', () => { // A sibling whose name merely starts with the root is not under it. expect(moduleIdFor('srcx/y.ts', 'src', 1)).toBeNull(); }); + +describe('folders nothing forks in (Maven, Gradle)', () => { + const MAVEN = [ + 'src/main/java/org/springframework/samples/petclinic/PetClinicApplication.java', + 'src/main/java/org/springframework/samples/petclinic/owner/Owner.java', + 'src/main/java/org/springframework/samples/petclinic/owner/OwnerController.java', + 'src/main/java/org/springframework/samples/petclinic/vet/Vet.java', + 'src/main/java/org/springframework/samples/petclinic/visit/Visit.java', + 'src/main/java/org/springframework/samples/petclinic/model/BaseEntity.java', + 'src/test/java/org/springframework/samples/petclinic/owner/OwnerControllerTests.java', + ]; + + it('finds the folders with one subfolder and no file of their own', () => { + expect([...passThroughDirs(MAVEN)].sort()).toEqual([ + 'src/main', + 'src/main/java', + 'src/main/java/org', + 'src/main/java/org/springframework', + 'src/main/java/org/springframework/samples', + 'src/test', + 'src/test/java', + 'src/test/java/org', + 'src/test/java/org/springframework', + 'src/test/java/org/springframework/samples', + 'src/test/java/org/springframework/samples/petclinic', + ]); + // A folder holding a file is a boundary even with one subfolder. + expect(passThroughDirs(['a/b/c.ts', 'a/x.ts']).has('a')).toBe(false); + }); + + it('counts depth in folders that fork, and elides the chain in the label', () => { + const through = passThroughDirs(MAVEN); + expect(moduleIdFor(MAVEN[1]!, 'src', 1, through)).toEqual({ + id: 'src/main/java/org/springframework/samples/petclinic', + label: 'src/main/…/petclinic', + facade: false, + }); + expect(moduleIdFor(MAVEN[1]!, 'src', 2, through)).toEqual({ + id: 'src/main/java/org/springframework/samples/petclinic/owner', + label: 'src/main/…/petclinic/owner', + facade: false, + }); + // The application class sits loose in the package root. + expect(moduleIdFor(MAVEN[0]!, 'src', 2, through)).toEqual({ + id: 'src/main/java/org/springframework/samples/petclinic/(root files)', + label: 'src/main/…/petclinic/(root files)', + facade: false, + }); + // Without the set, nothing changes: one level per folder. + expect(moduleIdFor(MAVEN[1]!, 'src', 2)?.id).toBe('src/main/java'); + }); + + it('opens a Maven project on its packages, not on one box', () => { + const files = MAVEN.map((p) => ({ path: p, symbols: 10, test: p.includes('/test/') })); + const through = passThroughDirs(MAVEN); + const depth = pickDefaultDepth(files, 'src', through); + const ids = new Set(files.filter((f) => !f.test).map((f) => moduleIdFor(f.path, 'src', depth, through)!.id)); + expect(depth).toBe(2); + expect([...ids].sort()).toEqual([ + 'src/main/java/org/springframework/samples/petclinic/(root files)', + 'src/main/java/org/springframework/samples/petclinic/model', + 'src/main/java/org/springframework/samples/petclinic/owner', + 'src/main/java/org/springframework/samples/petclinic/vet', + 'src/main/java/org/springframework/samples/petclinic/visit', + ]); + }); +}); }); describe('normalizeRoot', () => { diff --git a/docs/viewer-launch-changelog.md b/docs/viewer-launch-changelog.md index ba98fcfa..910fb905 100644 --- a/docs/viewer-launch-changelog.md +++ b/docs/viewer-launch-changelog.md @@ -161,6 +161,8 @@ These describe `codegraph ui` and its screens. They were taken out of `## [Unrel ## Fixes — Symbols, tests and the viewer +- **The Map opens a Maven or Gradle project on its packages.** A Java project keeps every file under `src/main/java/org///`, and those folders hold nothing but the next one, so the Map drew the whole program as one `src/main/java/org` box and no grouping option reached further. A folder with one subfolder and no files of its own no longer counts as a level: petclinic opens on `owner`, `vet`, `model` and `system`, each labelled `src/main/java/…/petclinic/owner`, with the full path on hover. + - In `codegraph ui`, routes whose handlers live in more than 60 different files are all linked to their handler, instead of the later ones showing "not in the index". (#1975) - The viewer continues to count module-level initializer calls as top-level file activity in entry points and file screens. diff --git a/src/ui-server/api/map.ts b/src/ui-server/api/map.ts index b593e86b..a78f63e7 100644 --- a/src/ui-server/api/map.ts +++ b/src/ui-server/api/map.ts @@ -106,7 +106,11 @@ export function rootFilesId(root: string): string { export interface WireMapModule { /** Directory path, or the `(root files)` bucket, or a façade file's own path. */ id: string; - /** Last path segment — what the node label shows when the id is long. */ + /** + * What the box says: the id, with a folder chain the repository never forks + * in written `first/…/last` — Maven's `src/main/java/…/petclinic/owner` + * rather than all seven segments. Equal to `id` everywhere else. + */ label: string; files: number; symbols: number; @@ -222,18 +226,82 @@ function stemOf(basename: string): string { return dot <= 0 ? basename : basename.slice(0, dot); } +/** + * Directories a module boundary never falls on: exactly one subdirectory and + * no file of their own. A Maven project keeps every line of Java under + * `src/main/java/org/springframework/samples/petclinic/`, and the first four + * of those folders split nothing — cutting at any of them draws the whole + * program as one box, and no depth the reader can pick gets past them. Such a + * folder joins the level below it, so depth counts folders that fork. + * + * Read from every indexed file, tests included, so a module's id does not + * change when the reader toggles tests. The repository root is never one. + */ +export function passThroughDirs(paths: Iterable): Set { + const children = new Map>(); + const holdsFiles = new Set(); + for (const raw of paths) { + const parts = toPosixPath(raw).split('/').filter(Boolean); + let dir = ''; + for (let i = 0; i < parts.length - 1; i++) { + let kids = children.get(dir); + if (!kids) children.set(dir, (kids = new Set())); + kids.add(parts[i]!); + dir = dir ? `${dir}/${parts[i]}` : parts[i]!; + } + holdsFiles.add(dir); + } + const out = new Set(); + for (const [dir, kids] of children) { + if (dir !== '' && kids.size === 1 && !holdsFiles.has(dir)) out.add(dir); + } + return out; +} + +/** + * Where each level of a file's directory path ends, as indexes into `dirs`: a + * level is one folder plus every pass-through folder that follows it. + */ +function levelEnds(root: string, dirs: readonly string[], passThrough: ReadonlySet | undefined): number[] { + const ends: number[] = []; + for (let i = 0; i < dirs.length; ) { + i++; + while (passThrough && i < dirs.length && passThrough.has(joinPath(root, dirs.slice(0, i)))) i++; + ends.push(i); + } + return ends; +} + +function joinPath(root: string, segments: readonly string[]): string { + return [root, ...segments].filter(Boolean).join('/'); +} + +/** The first `levels` levels of `dirs`, a chain of three or more folders written `first/…/last`. */ +function levelLabel(root: string, dirs: readonly string[], ends: readonly number[]): string { + const out: string[] = root ? [root] : []; + let from = 0; + for (const end of ends) { + const chain = dirs.slice(from, end); + out.push(chain.length >= 3 ? `${chain[0]}/…/${chain[chain.length - 1]}` : chain.join('/')); + from = end; + } + return out.join('/'); +} + /** * Which module a file belongs to, or `null` when it is outside the root. * - * `depth` segments under the root name the module. A file with fewer segments - * than that is loose in the root: a façade keeps its own box, everything else - * joins the `(root files)` bucket. + * `depth` levels under the root name the module — a level being a folder and + * the {@link passThroughDirs} that follow it (without them, one level per + * folder). A file with fewer levels than that is loose in the root: a façade + * keeps its own box, everything else joins the `(root files)` bucket. */ export function moduleIdFor( filePath: string, root: string, - depth: number -): { id: string; facade: boolean } | null { + depth: number, + passThrough?: ReadonlySet +): { id: string; facade: boolean; label: string } | null { const path = toPosixPath(filePath); let rel = path; if (root) { @@ -242,16 +310,24 @@ export function moduleIdFor( } const parts = rel.split('/').filter(Boolean); if (parts.length === 0) return null; - if (parts.length <= depth) { + const dirs = parts.slice(0, -1); + const ends = levelEnds(root, dirs, passThrough); + if (ends.length < depth) { // A loose file. The directories it DOES have still qualify it, so // `src/a/b.ts` at depth 2 lands in `src/a/(root files)`, not the top one. - const dir = [root, ...parts.slice(0, -1)].filter(Boolean).join('/'); - if (FACADE_STEMS.has(stemOf(parts[parts.length - 1] ?? ''))) { - return { id: [root, ...parts].filter(Boolean).join('/'), facade: true }; + const dir = joinPath(root, dirs); + const dirLabel = levelLabel(root, dirs, ends); + const file = parts[parts.length - 1] ?? ''; + if (FACADE_STEMS.has(stemOf(file))) { + return { id: joinPath(root, parts), facade: true, label: dirLabel ? `${dirLabel}/${file}` : file }; } - return { id: rootFilesId(dir), facade: false }; + return { id: rootFilesId(dir), facade: false, label: rootFilesId(dirLabel) }; } - return { id: [root, ...parts.slice(0, depth)].filter(Boolean).join('/'), facade: false }; + return { + id: joinPath(root, dirs.slice(0, ends[depth - 1])), + facade: false, + label: levelLabel(root, dirs, ends.slice(0, depth)), + }; } /** @@ -318,13 +394,14 @@ const MAX_MODULES = 60; function tallyModules( files: ReadonlyArray<{ path: string; symbols: number; test: boolean }>, root: string, - depth: number + depth: number, + passThrough?: ReadonlySet ): { count: number; share: number; largestFiles: number } { const byModule = new Map(); let total = 0; for (const file of files) { if (file.test) continue; - const assigned = moduleIdFor(file.path, root, depth); + const assigned = moduleIdFor(file.path, root, depth, passThrough); if (assigned === null) continue; let entry = byModule.get(assigned.id); if (!entry) byModule.set(assigned.id, (entry = { symbols: 0, files: 0 })); @@ -364,7 +441,8 @@ function tallyModules( */ export function pickDefaultDepth( files: ReadonlyArray<{ path: string; symbols: number; test: boolean }>, - root: string + root: string, + passThrough?: ReadonlySet ): number { // Past the deepest directory, a bigger number only renames boxes to // `src/a/(root files)`. There is nothing below the leaves. @@ -374,13 +452,14 @@ export function pickDefaultDepth( const path = toPosixPath(file.path); if (root && !path.startsWith(`${root}/`)) continue; const rel = root ? path.slice(root.length + 1) : path; - deepest = Math.max(deepest, rel.split('/').filter(Boolean).length - 1); + const dirs = rel.split('/').filter(Boolean).slice(0, -1); + deepest = Math.max(deepest, levelEnds(root, dirs, passThrough).length); } let fallback = DEFAULT_DEPTH; let fallbackCount = 0; for (let depth = DEFAULT_DEPTH; depth <= Math.min(MAX_DEPTH, deepest); depth += 1) { - const tally = tallyModules(files, root, depth); + const tally = tallyModules(files, root, depth, passThrough); if (tally.count === 0) break; // Deeper only gets more crowded from here. if (tally.count > MAX_MODULES) break; @@ -494,10 +573,11 @@ export function buildMap(cg: CodeGraph, projectRoot: string, query: URLSearchPar }); const root = requestedRoot ?? pickDefaultRoot(fileRecords); + const passThrough = passThroughDirs(fileRecords.map((f) => f.path)); // Root first, then depth against THAT root: how finely to cut depends on // what is being cut. Choosing `src` and then asking for one level under it // is the same question as choosing the whole project and asking for two. - const depth = requestedDepth ?? pickDefaultDepth(fileRecords, root); + const depth = requestedDepth ?? pickDefaultDepth(fileRecords, root, passThrough); const stats = cg.getStats(); const key = [ projectRoot, @@ -531,12 +611,18 @@ export function buildMap(cg: CodeGraph, projectRoot: string, query: URLSearchPar >(); const moduleOfFile = new Map(); - const assigned = new Map(); + const assigned = new Map(); + const labelOf = new Map(); for (const file of fileRecords) { - const at = moduleIdFor(file.path, root, depth); - if (at !== null) assigned.set(file.path, at); + const at = moduleIdFor(file.path, root, depth, passThrough); + if (at === null) continue; + assigned.set(file.path, at); + labelOf.set(at.id, at.label); } const renamed = collapseLoneRootFiles(new Set([...assigned.values()].map((a) => a.id))); + for (const [from, to] of renamed) { + labelOf.set(to, (labelOf.get(from) ?? to).replace(/\/\(root files\)$/, '')); + } for (const file of fileRecords) { const at = assigned.get(file.path); @@ -625,7 +711,7 @@ export function buildMap(cg: CodeGraph, projectRoot: string, query: URLSearchPar const shown = entry.paths.slice().sort().slice(0, MAX_FILES_PER_MODULE); return { id: entry.id, - label: entry.id.slice(entry.id.lastIndexOf('/') + 1) || entry.id, + label: labelOf.get(entry.id) ?? entry.id, files: entry.files, symbols: entry.symbols, languages: [...entry.languages] diff --git a/ui/src/components/map/ModuleNode.svelte b/ui/src/components/map/ModuleNode.svelte index 9d2f2bbd..8748a21c 100644 --- a/ui/src/components/map/ModuleNode.svelte +++ b/ui/src/components/map/ModuleNode.svelte @@ -60,7 +60,8 @@ layout.generated ? '. Every file in it is tool-generated.' : '' }`} > - {module.id} + + {module.label || module.id} {moduleMetaLabel(module, layout.island)} { const island = islands.has(m.id); - const lines = options.sizing?.(m, island) ?? { label: m.id, meta: moduleMetaLabel(m, island) }; + const lines = options.sizing?.(m, island) ?? { label: m.label || m.id, meta: moduleMetaLabel(m, island) }; const count = portCount.get(m.id) ?? { top: 0, bottom: 0 }; const forPorts = (Math.max(count.top, count.bottom) + 1) * portPitch; return [m.id, Math.max(nodeWidth(lines.label, lines.meta), forPorts)]; diff --git a/ui/src/lib/wire.ts b/ui/src/lib/wire.ts index 95d04f09..227989e6 100644 --- a/ui/src/lib/wire.ts +++ b/ui/src/lib/wire.ts @@ -596,6 +596,7 @@ export interface WireFlowPayload { export interface WireMapModule { /** Directory path, the `(root files)` bucket, or a façade file's own path. */ id: string; + /** What the box says: the id, a folder chain nothing forks in written `first/…/last`. */ label: string; files: number; symbols: number;