mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-02 01:37:32 +08:00
fix(ui-map): count depth in folders that fork, so a Maven project opens on its packages (#2102)
A Maven / Gradle project keeps every line of Java under src/main/java/org/<company>/<app>/, 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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
0419a51636
commit
dcd79ad3c5
@@ -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', () => {
|
||||
|
||||
@@ -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/<company>/<app>/`, 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.
|
||||
|
||||
+108
-22
@@ -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<string>): Set<string> {
|
||||
const children = new Map<string, Set<string>>();
|
||||
const holdsFiles = new Set<string>();
|
||||
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<string>();
|
||||
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<string> | 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<string>
|
||||
): { 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<string>
|
||||
): { count: number; share: number; largestFiles: number } {
|
||||
const byModule = new Map<string, { symbols: number; files: number }>();
|
||||
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<string>
|
||||
): 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<string, string>();
|
||||
|
||||
const assigned = new Map<string, { id: string; facade: boolean }>();
|
||||
const assigned = new Map<string, { id: string; facade: boolean; label: string }>();
|
||||
const labelOf = new Map<string, string>();
|
||||
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]
|
||||
|
||||
@@ -60,7 +60,8 @@
|
||||
layout.generated ? '. Every file in it is tool-generated.' : ''
|
||||
}`}
|
||||
>
|
||||
<span class="name">{module.id}</span>
|
||||
<!-- The label elides a folder chain nothing forks in; the title keeps the full path. -->
|
||||
<span class="name">{module.label || module.id}</span>
|
||||
<!-- The same string nodeWidth() sized the box for; they must not drift. -->
|
||||
<span class="count" class:island={layout.island}
|
||||
>{moduleMetaLabel(module, layout.island)}</span
|
||||
|
||||
@@ -414,7 +414,7 @@ export function buildMapLayout(
|
||||
const widths = new Map(
|
||||
modules.map((m) => {
|
||||
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)];
|
||||
|
||||
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user