fix(class): keep every multiplicity and role at its own end (#593)

Sol's library build lost two of five samples on data accuracy, and three on
the class-drawn check, after #588: Member to Reservation runs straight up in
one run, the "reservations" role found no spot clearly its own line's at
Reservation, and walked down that run until it stood just above Member.

A multiplicity or role now only takes a spot beside its own end: its middle
within 110 units of the end's tip (the class-drawn check's reach) and the
line's other tip half again as far off as its own, and 8 units further. That
also keeps a self-loop's labels out from between its two runs. With no spot
both beside its end and clearly its own line's, it stays beside its end, and
the placements run again keeping room for labels; one that reads at most 5%
longer than the placement it replaces may now take over.

The class fuzz gains the end invariant: main misplaced 17 end labels across
the 240-model full run and 74 across eight more seeds, none now. On those
eight seeds, 8 labels whose room would cost crossings now stand at their end
but nearer a neighboring line (81 findings on main, 12 now). Across the
100 saved class builds from the evals, three scenes change, each only in that
one end's labels, and each now passes the class-drawn check. The renderer
test checks every class fixture, and a new fixture and story carry Sol's
build.

Fixes #589

Author: anandpant

packages/diagram/core/src/class-fixtures.ts

@@ -12,8 +12,9 @@ const build = (input: Record<string, unknown>): ClassDiagram =>
  * Class diagrams that stress the layout: a model of twenty classes, a
  * hierarchy wider than any row should run, one five levels deep with an
  * interface several rows above its implementer, a class related to itself
- * twice, two classes in one row with a third between them, and two named
- * dependencies running opposite ways between a pair of interfaces.
+ * twice, two classes in one row with a third between them, two named
+ * dependencies running opposite ways between a pair of interfaces, and a
+ * library whose roles must each stay at their own end.
  */
 export const classStressFixtures = {
 	library: build({
@@ -364,4 +365,158 @@ export const classStressFixtures = {
 			},
 		],
 	}),
+	// Sol's library-system-class-v2 build (r30): the "reservations" role at the Reservation end
+	// of Member - Reservation, a line of one run, stood beside Member instead.
+	memberReservations: build({
+		id: "library-system",
+		title: "Library system",
+		classes: [
+			{
+				id: "Library",
+				attributes: ["- name: String", "- branches: List<Branch>", "- catalog: Catalog"],
+				methods: ["+ search(query: String): List<Item>"],
+			},
+			{ id: "Branch", attributes: ["- address: String"] },
+			{
+				id: "Catalog",
+				attributes: ["- items: List<Item>"],
+				methods: ["+ add(item: Item): void", "+ find(isbn: String): Book"],
+			},
+			{
+				id: "Item",
+				kind: "abstract",
+				attributes: ["# id: UUID", "# title: String"],
+				methods: ["+ loanPeriod(): int {abstract}"],
+			},
+			{
+				id: "Book",
+				attributes: ["- isbn: String", "- pages: int", "- authors: List<Author>"],
+				methods: ["+ loanPeriod(): int"],
+			},
+			{ id: "DVD", attributes: ["- minutes: int"], methods: ["+ loanPeriod(): int"] },
+			{ id: "Magazine", attributes: ["- issue: int"], methods: ["+ loanPeriod(): int"] },
+			{ id: "Author", attributes: ["- name: String", "- books: List<Book>"] },
+			{
+				id: "Member",
+				attributes: [
+					"- memberId: String",
+					"- name: String",
+					"- loans: List<Loan>",
+					"- reservations: List<Reservation>",
+				],
+				methods: ["+ borrow(item: Item): Loan", "+ reserve(item: Item): Reservation"],
+			},
+			{ id: "StudentMember", attributes: ["- school: String"] },
+			{ id: "StaffMember", attributes: ["- department: String"] },
+			{
+				id: "Loan",
+				attributes: ["- due: Date", "- returned: Date", "- item: Item", "- fine: Fine"],
+				methods: ["+ isOverdue(): boolean"],
+			},
+			{ id: "Reservation", attributes: ["- placed: Date", "- item: Item"] },
+			{ id: "Fine", attributes: ["- amount: Money"], methods: ["+ pay(): void"] },
+			{
+				id: "Librarian",
+				attributes: ["- staffId: String", "- branch: Branch"],
+				methods: ["+ checkOut(loan: Loan): void"],
+			},
+		],
+		relationships: [
+			{
+				id: "relationship-1",
+				kind: "composition",
+				from: "Library",
+				to: "Branch",
+				fromMultiplicity: "1",
+				toMultiplicity: "1..*",
+				toRole: "branches",
+			},
+			{
+				id: "relationship-2",
+				kind: "composition",
+				from: "Library",
+				to: "Catalog",
+				fromMultiplicity: "1",
+				toMultiplicity: "1",
+				toRole: "catalog",
+			},
+			{
+				id: "relationship-3",
+				kind: "composition",
+				from: "Loan",
+				to: "Fine",
+				fromMultiplicity: "1",
+				toRole: "fine",
+			},
+			{
+				id: "relationship-4",
+				kind: "aggregation",
+				from: "Catalog",
+				to: "Item",
+				toMultiplicity: "0..*",
+				toRole: "items",
+			},
+			{ id: "relationship-5", kind: "inheritance", from: "Book", to: "Item" },
+			{ id: "relationship-6", kind: "inheritance", from: "DVD", to: "Item" },
+			{ id: "relationship-7", kind: "inheritance", from: "Magazine", to: "Item" },
+			{
+				id: "relationship-8",
+				kind: "association",
+				from: "Book",
+				to: "Author",
+				fromMultiplicity: "0..*",
+				toMultiplicity: "1..*",
+				fromRole: "books",
+				toRole: "authors",
+			},
+			{ id: "relationship-9", kind: "inheritance", from: "StudentMember", to: "Member" },
+			{ id: "relationship-10", kind: "inheritance", from: "StaffMember", to: "Member" },
+			{
+				id: "relationship-11",
+				kind: "association",
+				from: "Member",
+				to: "Loan",
+				toMultiplicity: "0..*",
+				toRole: "loans",
+				navigable: true,
+			},
+			{
+				id: "relationship-12",
+				kind: "association",
+				from: "Member",
+				to: "Reservation",
+				toMultiplicity: "0..*",
+				toRole: "reservations",
+				navigable: true,
+			},
+			{
+				id: "relationship-13",
+				kind: "association",
+				from: "Loan",
+				to: "Item",
+				toRole: "item",
+				navigable: true,
+			},
+			{
+				id: "relationship-14",
+				kind: "association",
+				from: "Reservation",
+				to: "Item",
+				toRole: "item",
+				navigable: true,
+			},
+			{
+				id: "relationship-15",
+				kind: "association",
+				from: "Librarian",
+				to: "Branch",
+				toRole: "branch",
+				navigable: true,
+			},
+			{ id: "relationship-16", kind: "dependency", from: "Librarian", to: "Loan" },
+			{ id: "relationship-17", kind: "dependency", from: "Library", to: "Item" },
+			{ id: "relationship-18", kind: "dependency", from: "Catalog", to: "Book" },
+			{ id: "relationship-19", kind: "dependency", from: "Member", to: "Item" },
+		],
+	}),
 } as const satisfies Record<string, ClassDiagram>;

packages/diagram/excalidraw/src/lib/class-fuzz.test.ts

@@ -205,6 +205,8 @@ const PARALLEL_CLEARANCE = 10;
  */
 const ATTRIBUTION_RATIO = 1.5;
 const ATTRIBUTION_MARGIN = 8;
+/** A multiplicity or role sits beside its end when its middle stands this near the end's tip. */
+const END_LABEL_REACH = 110;
 
 /** Everything the export validator, the canvas validator or the layout guards would flag. */
 function defects(diagram: ClassDiagram): string[] {
@@ -349,6 +351,29 @@ function defects(diagram: ClassDiagram): string[] {
 			? [`${label.id} ${own.toFixed(0)} px from its line, ${nearest.toFixed(0)} px from ${other}`]
 			: [];
 	});
+	// Every multiplicity and role reads as its own end's: beside that end's tip, and the
+	// line's other tip clearly further off.
+	const tips = new Map(arrows.map((arrow) => [arrow.edgeId, arrow.points]));
+	const reach = (label: (typeof labels)[number], tip: { x: number; y: number }) =>
+		Math.hypot(
+			Math.max(0, label.left - tip.x, tip.x - label.right),
+			Math.max(0, label.top - tip.y, tip.y - label.bottom),
+		);
+	const misplaced = labels.flatMap((label) => {
+		const [, owner, end] = label.id.split(":");
+		if (end !== "from" && end !== "to") return [];
+		const points = tips.get(owner!)!;
+		const [tip, far] = end === "from" ? [points[0], points.at(-1)!] : [points.at(-1)!, points[0]];
+		const [own, other] = [reach(label, tip), reach(label, far)];
+		const center = Math.hypot(
+			(label.left + label.right) / 2 - tip.x,
+			(label.top + label.bottom) / 2 - tip.y,
+		);
+		return center > END_LABEL_REACH ||
+			other < Math.max(ATTRIBUTION_RATIO * own, own + ATTRIBUTION_MARGIN)
+			? [`${label.id} ${own.toFixed(0)} px from its end, ${other.toFixed(0)} px from the other`]
+			: [];
+	});
 	// No two lines run side by side closer than a reader can tell apart.
 	const runs = arrows.flatMap((arrow) =>
 		segmentsFromPoints(arrow.points).map((segment) => ({ arrow: arrow.id, segment })),
@@ -378,6 +403,7 @@ function defects(diagram: ClassDiagram): string[] {
 		...unwrapped,
 		...crowded,
 		...misattributed,
+		...misplaced,
 		...hugging,
 	];
 }

packages/diagram/renderer/src/class-diagram.ts

@@ -61,6 +61,12 @@ const LABEL_CLEARANCE = 3;
  */
 const ATTRIBUTION_RATIO = 1.5;
 const ATTRIBUTION_MARGIN = 8;
+/**
+ * A multiplicity or role reads as its own end's only when its middle stands
+ * this near the end's tip and, by the rule above, the line's other tip
+ * stands clearly further off.
+ */
+const END_LABEL_REACH = 110;
 /** Past this, a layout's export reads poorly; a candidate pays for any excess. */
 const READABLE_ASPECT = 2.5;
 const NAME_FILL = "#efe6ec";
@@ -73,6 +79,11 @@ const EXPORT_PADDING = 20;
  * placement within it that routes no worse takes its place.
  */
 const LONG_EXPORT = 2.4;
+/**
+ * A placement keeping room for labels that reads longer than that may still
+ * take over, when it reads at most this much longer than the one it replaces.
+ */
+const LABEL_ROOM_STRETCH = 1.05;
 
 /** Each kind's line: dashed or solid, and the heads at its `from` and `to` ends. */
 const LINE_STYLES: Readonly<
@@ -451,7 +462,7 @@ function bestScene(
 	}
 	// While a label stands clearly its own line's nowhere, the placements run again
 	// keeping room for labels; one takes its place when it leaves fewer such labels,
-	// draws no more crossings, detours or hugging runs, reads no longer, and draws
+	// draws no more crossings, detours or hugging runs, reads hardly longer, and draws
 	// no more crossings and findings together than one more for each label it clears:
 	// a label read as another line's misleads a reader more than a jog does.
 	const labelled = (routed: Routed) => ({
@@ -475,7 +486,7 @@ function bestScene(
 		const was = chosen.counts;
 		if (
 			routed.labels.unclear < chosen.labels.unclear &&
-			(!long(routed) || exportAspect(scene) <= exportAspect(chosen.scene)) &&
+			(!long(routed) || exportAspect(scene) <= LABEL_ROOM_STRETCH * exportAspect(chosen.scene)) &&
 			counts.crossings <= was.crossings &&
 			counts.crossings + counts.findings <=
 				was.crossings + was.findings + chosen.labels.unclear - routed.labels.unclear &&
@@ -635,6 +646,13 @@ function* labelSpots(
 	}
 }
 
+function rectPointDistance(rect: Rect, point: { readonly x: number; readonly y: number }): number {
+	return Math.hypot(
+		Math.max(0, rect.left - point.x, point.x - rect.right),
+		Math.max(0, rect.top - point.y, point.y - rect.bottom),
+	);
+}
+
 /** A route from its start up to halfway along it. */
 function halfway(points: readonly { x: number; y: number }[]): { x: number; y: number }[] {
 	const lengths = points
@@ -659,7 +677,9 @@ function halfway(points: readonly { x: number; y: number }[]): { x: number; y: n
  * Place each multiplicity and role beside its end's first run, the two on
  * opposite sides of the line, and each relationship's name beside its
  * longest run, every one clear of the boxes, the lines and the others, and
- * clearly nearer its own line than any other, so none reads as another's.
+ * clearly nearer its own line than any other, so none reads as another's;
+ * a multiplicity or role also stays beside its own end's tip, clearly nearer
+ * it than the line's other tip, so none reads as the other end's.
  */
 function placeLabels(
 	diagram: ClassDiagram,
@@ -683,19 +703,23 @@ function placeLabels(
 		placed.filter((other) => rectsOverlap(rect, other, LABEL_CLEARANCE)).length;
 	const free = (rect: Rect) => collisions(rect) === 0;
 	const anywhere = () => free;
-	// Free, and the text as drawn within its room stands clearly nearer its own line.
-	const attributed = (owner: string, text: string) => (rect: Rect) => {
-		if (!free(rect)) return false;
+	// The text as drawn within its room.
+	const drawnRect = (text: string, rect: Rect): Rect => {
 		const inset = {
 			x: (labelWidth(text) - estimateTextWidth(text, LABEL_FONT_SIZE)) / 2,
 			y: (text.split("\n").length * (LABEL_HEIGHT - LABEL_FONT_SIZE * 1.35)) / 2,
 		};
-		const drawn = {
+		return {
 			left: rect.left + inset.x,
 			top: rect.top + inset.y,
 			right: rect.right - inset.x,
 			bottom: rect.bottom - inset.y,
 		};
+	};
+	// Free, and the text as drawn within its room stands clearly nearer its own line.
+	const attributed = (owner: string, text: string) => (rect: Rect) => {
+		if (!free(rect)) return false;
+		const drawn = drawnRect(text, rect);
 		let [own, other] = [Infinity, Infinity];
 		for (const run of owned) {
 			const distance = rectSegmentDistance(drawn, run.segment);
@@ -741,6 +765,7 @@ function placeLabels(
 			const labels = endLabels(relationship, end);
 			if (labels.length === 0) continue;
 			const route = end === "from" ? arrow.points : arrow.points.toReversed();
+			const [tip, far] = [route[0]!, route.at(-1)!];
 			const owner = end === "from" ? arrow.sourceNodeId : arrow.targetNodeId;
 			const keys = [
 				relationship[end === "from" ? "fromMultiplicity" : "toMultiplicity"] && "multiplicity",
@@ -763,10 +788,29 @@ function placeLabels(
 				placed.splice(placed.length - rects.length, rects.length);
 				return rects.length === labels.length ? rects : undefined;
 			};
-			// On the end's first run, clearly its own line's: two labels on opposite
-			// sides, whichever way round fits, else together on one side; one label on
-			// either side, nearest the end first.
-			const mine = (text: string) => attributed(relationship.id, text);
+			// The text as drawn stands beside this end's tip, the other tip clearly further off.
+			const beside = (text: string) => (rect: Rect) => {
+				const drawn = drawnRect(text, rect);
+				const [own, other] = [rectPointDistance(drawn, tip), rectPointDistance(drawn, far)];
+				return (
+					Math.hypot(
+						(drawn.left + drawn.right) / 2 - tip.x,
+						(drawn.top + drawn.bottom) / 2 - tip.y,
+					) <= END_LABEL_REACH &&
+					other >= Math.max(ATTRIBUTION_RATIO * own, own + ATTRIBUTION_MARGIN)
+				);
+			};
+			const near = (text: string) => {
+				const here = beside(text);
+				return (rect: Rect) => free(rect) && here(rect);
+			};
+			// On the end's first run, beside the end and clearly its own line's: two labels
+			// on opposite sides, whichever way round fits, else together on one side; one
+			// label on either side, nearest the end first.
+			const mine = (text: string) => {
+				const [line, here] = [attributed(relationship.id, text), beside(text)];
+				return (rect: Rect) => line(rect) && here(rect);
+			};
 			const sides: ((index: number) => readonly (-1 | 1)[])[] =
 				labels.length === 2
 					? [
@@ -777,23 +821,27 @@ function placeLabels(
 						]
 					: [() => [1, -1]];
 			// Failing that, each the nearest such spot on either side along the end's
-			// half of the line; then opposite sides, then each the nearest free spot on
-			// either side anywhere along the line; then, with none free, its first spot.
+			// half of the line; then the same beside the end, if nearer another line;
+			// then opposite sides, then each the nearest free spot on either side along
+			// the end's half; then, with none free, its first spot.
 			const orders: (readonly (-1 | 1)[])[] =
 				labels.length === 2
 					? [
 							[1, -1],
 							[-1, 1],
 						]
 					: [[1], [-1]];
-			const clear =
-				sides.map((side) => place(route.slice(0, 2), side, mine)).find(Boolean) ??
-				place(halfway(route), () => [1, -1], mine);
+			const half = halfway(route);
+			const nearest = (accept: (text: string) => (rect: Rect) => boolean) =>
+				sides.map((side) => place(route.slice(0, 2), side, accept)).find(Boolean) ??
+				place(half, () => [1, -1], accept);
+			const clear = nearest(mine);
 			if (!clear) unclear += labels.length;
 			const chosen =
 				clear ??
-				orders.map((order) => place(route, (index) => [order[index]!], anywhere)).find(Boolean) ??
-				place(route, () => [1, -1], anywhere) ??
+				nearest(near) ??
+				orders.map((order) => place(half, (index) => [order[index]!], anywhere)).find(Boolean) ??
+				place(half, () => [1, -1], anywhere) ??
 				labels.map((label) => spots(route, labelWidth(label), LABEL_HEIGHT, [1, -1], 4)[0]!);
 			for (const [index, rect] of chosen.entries())
 				add(