Skip to content

Commit a46a4e0

Browse files
valuecodesclaude
andcommitted
fix: address PR review feedback
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 755efac commit a46a4e0

3 files changed

Lines changed: 53 additions & 12 deletions

File tree

‎packages/code-graph/src/entrypoints.ts‎

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,20 @@ const MAX_API_SCORE = 3;
1919
/**
2020
* How much a module looks like a way in: +3 for an entry name (`index`,
2121
* `main`, `server`, `app`, `cli`, any extension), +1 per exported function
22-
* nobody calls (at most 3), -1 per directory below a leading `src/`.
22+
* nobody calls (at most 3), -1 per directory below a leading `src/`, the
23+
* path taken from `root` (a package's directory; `""` for the repository).
2324
*/
2425
const entryScore = (
2526
module: ModuleNode,
2627
graph: CodeGraph,
27-
called: ReadonlySet<string>
28+
called: ReadonlySet<string>,
29+
root: string
2830
): number => {
29-
const segments = module.id.split("/");
31+
const path =
32+
root !== "" && module.id.startsWith(`${root}/`)
33+
? module.id.slice(root.length + 1)
34+
: module.id;
35+
const segments = path.split("/");
3036
const basename = segments.at(-1) ?? "";
3137
const directories = segments.slice(0, -1);
3238
const depth =
@@ -54,14 +60,16 @@ const byPath = (a: ModuleNode, b: ModuleNode): number => {
5460
* The modules the entrance leads to, best first, among `modules` (all of
5561
* the graph's by default; a zone's for its hub), counting only imports and
5662
* calls between those. Every module nobody imports is one (nothing else
57-
* leads there), ranked by `entryScore`, ties by path. Modules only cycles
58-
* reach then add the best of what is still out of reach, one at a time,
59-
* until imports from the entries reach every module. Never empty for a
60-
* non-empty `modules`.
63+
* leads there), ranked by `entryScore` with paths taken from `root` (the
64+
* zone's package directory), ties by path. Modules only cycles reach then
65+
* add the best of what is still out of reach, one at a time, until imports
66+
* from the entries reach every module. Never empty for a non-empty
67+
* `modules`.
6168
*/
6269
const entryModules = (
6370
graph: CodeGraph,
64-
modules: readonly ModuleNode[] = graph.modules
71+
modules: readonly ModuleNode[] = graph.modules,
72+
root = ""
6573
): readonly ModuleNode[] => {
6674
const inside = new Set(modules.map((module) => module.id));
6775
const links = new Map(
@@ -81,7 +89,10 @@ const entryModules = (
8189
)
8290
);
8391
const scores = new Map(
84-
modules.map((module) => [module.id, entryScore(module, graph, called)])
92+
modules.map((module) => [
93+
module.id,
94+
entryScore(module, graph, called, root),
95+
])
8596
);
8697
const ranked = (group: readonly ModuleNode[]): readonly ModuleNode[] =>
8798
group.toSorted(

‎packages/code-graph/src/zone-graph.test.ts‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { generateWorld } from "@repo/world-generator";
33
import { validateGraph } from "@repo/world-generator/graph";
44
import { describe, expect, it } from "vitest";
55

6-
import { fixtureGraph, packageNode, withPackages } from "./fixture";
6+
import { fixtureGraph, importOf, packageNode, withPackages } from "./fixture";
77
import { ENTRANCE_ID, modulePortalId } from "./ids";
88
import { zoneEntranceGraph, zoneGraph } from "./zone-graph";
99
import { zonesOf } from "./zones";
@@ -131,6 +131,35 @@ describe("zoneGraph", () => {
131131
placesEverything(world);
132132
});
133133

134+
it("scores entry modules by their path inside the package", () => {
135+
const graph = withPackages(
136+
fixtureGraph(
137+
{
138+
path: "packages/lib/helper.ts",
139+
functions: [],
140+
imports: [importOf("packages/lib/src/api.ts")],
141+
},
142+
{
143+
path: "packages/lib/src/api.ts",
144+
functions: [{ name: "fetchAll", exported: true }],
145+
imports: [importOf("packages/lib/helper.ts")],
146+
},
147+
{ path: "apps/web/main.ts", functions: [] }
148+
),
149+
packageNode("@repo/lib", "packages/lib"),
150+
packageNode("web", "apps/web")
151+
);
152+
const lib = zonesOf(graph).find((zone) => zone.name === "@repo/lib");
153+
if (lib === undefined) {
154+
throw new Error("Expected a zone for @repo/lib");
155+
}
156+
// In the cycle, src/api.ts (API, no depth inside the package) beats
157+
// helper.ts; scored from the repository root they would tie on depth.
158+
expect((zoneGraph(graph, lib).portals ?? []).map((p) => p.to)).toEqual([
159+
"packages/lib/src/api.ts",
160+
]);
161+
});
162+
134163
it("keeps whole paths for the modules no package owns", () => {
135164
const graph = withPackages(
136165
fixtureGraph(

‎packages/code-graph/src/zone-graph.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,14 @@ const zoneEntranceGraph = (
2828
/**
2929
* A zone's hub chain, labelled with the package's name, holding a module
3030
* portal to each of the zone's entry modules (`entryModules` among its own
31-
* modules), each labelled with its path inside the package.
31+
* modules, scored from the package's directory), each labelled with its
32+
* path inside the package.
3233
*/
3334
const zoneGraph = (graph: CodeGraph, zone: Zone): WorldGraph =>
3435
hubChainGraph(
3536
zone.id,
3637
zone.name,
37-
entryModules(graph, zone.modules).map((module) => ({
38+
entryModules(graph, zone.modules, zone.pkg?.dir ?? "").map((module) => ({
3839
id: module.id,
3940
label:
4041
zone.pkg === null

0 commit comments

Comments
 (0)