Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions src/lib/components/PortTooltip.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@
<script lang="ts">
import type { InterfaceTemplate, InterfaceType } from "$lib/types";
import { getPortTooltipState } from "$lib/stores/portTooltip.svelte";
import { inferDirection } from "$lib/utils/port-utils";
import {
inferDirection,
inferSignalType,
getSignalLabel,
} from "$lib/utils/port-utils";

// Get reactive tooltip state from store
const tooltipState = $derived(getPortTooltipState());
Expand Down Expand Up @@ -68,6 +72,15 @@
return direction ? DIRECTION_LABELS[direction] : null;
}

// Signal label: explicit signal_type wins; otherwise inferred from the
// connector type plus the (explicit or inferred) direction. Null hides the row.
function getSignalTypeLabel(port: InterfaceTemplate): string | null {
const direction =
port.direction ?? inferDirection(port.type, port.mgmt_only);
const signal = port.signal_type ?? inferSignalType(port.type, direction);
return signal ? getSignalLabel(signal) : null;
Comment on lines +77 to +81

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: This computes signal text from an InterfaceTemplate only, so a placed port's signal_type override is never considered in the tooltip. That breaks the new precedence contract (placed override should win over template/inference) and will show incorrect signal labels for overridden ports. Update the tooltip input contract to include the placed port override (or a pre-resolved effective signal) and resolve as placed override → template value → inference. [api mismatch]

Severity Level: Major ⚠️
❌ Port tooltip shows wrong signal for overridden ports.
⚠️ Breaks documented precedence for PlacedPort signal override.
⚠️ Misleads layout users configuring explicit per-port signals.
Steps of Reproduction ✅
1. In `src/lib/components/PortIndicators.svelte:213-223`, each rendered port comes from
`portPositions` entries of the form `{ iface, port, x, y, color }`, where `port` is a
`PlacedPort` and `iface` is the `InterfaceTemplate`.

2. On hover, `handlePortMouseEnter` at `src/lib/components/PortIndicators.svelte:181-192`
calls `showPortTooltip(iface, rect.left + rect.width / 2, rect.top)`, passing only the
`InterfaceTemplate` and not the corresponding `PlacedPort`.

3. The tooltip store in `src/lib/stores/portTooltip.svelte.ts:8-23` defines
`PortTooltipState.port` as `InterfaceTemplate | null` and `showPortTooltip(port:
InterfaceTemplate, x: number, y: number)` assigns this template-only value to the store,
so the tooltip state never contains `PlacedPort` overrides.

4. The tooltip component `src/lib/components/PortTooltip.svelte:15-21` derives `port` from
`getPortTooltipState()` and `getSignalTypeLabel(port: InterfaceTemplate)` at lines 75-81
computes `signal` as `port.signal_type ?? inferSignalType(port.type, direction)`, meaning
it only considers the template `signal_type` field or inference; any `signal_type`
override stored on `PlacedPort` (as documented in `src/lib/types/index.ts:129-143` with
`signal_type?: SignalType` override) has no code path into the tooltip, so when a placed
port’s signal override differs from the template or inferred value, the tooltip still
shows the template/inferred signal and silently ignores the override.

Fix in Cursor Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** src/lib/components/PortTooltip.svelte
**Line:** 77:81
**Comment:**
	*Api Mismatch: This computes signal text from an `InterfaceTemplate` only, so a placed port's `signal_type` override is never considered in the tooltip. That breaks the new precedence contract (placed override should win over template/inference) and will show incorrect signal labels for overridden ports. Update the tooltip input contract to include the placed port override (or a pre-resolved effective signal) and resolve as placed override → template value → inference.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, but deferring for this PR. The port tooltip is template-only by existing design: the portTooltip store carries an InterfaceTemplate, and showPortTooltip is invoked with the template (see PortIndicators.svelte). The same gap already exists for the pre-existing PlacedPort.direction override, which getDirectionLabel likewise does not surface in the tooltip. This change deliberately mirrors that behavior; the issue scopes PlacedPort.signal_type as a read-time override, with the override-consuming read paths landing in the follow-up connections work.

The field is persisted and validated end to end: the v26.7.0-signal-type upgrade-corpus fixture round-trips an explicit AES3-over-XLR override. Threading placed-port overrides into the tooltip (for both signal_type and direction, to keep them consistent) is a reasonable follow-up rather than part of this additive-field change.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Customized review instruction saved!

Instruction:

Do not flag missing placed-port overrides in the port tooltip when the tooltip is intentionally template-only; keep tooltip reads aligned with the existing template-only contract and leave placed-port override threading for the follow-up read-path work.

Applied to:

  • src/lib/components/PortTooltip.svelte
  • src/lib/components/PortIndicators.svelte
  • src/lib/stores/portTooltip.svelte.ts

💡 To manage or update this instruction, visit: CodeAnt AI Settings

}

// Get PoE label
function getPoELabel(
poeMode?: "pd" | "pse",
Expand All @@ -94,6 +107,9 @@
{#if getDirectionLabel(port)}
<div class="port-tooltip-direction">{getDirectionLabel(port)}</div>
{/if}
{#if getSignalTypeLabel(port)}
<div class="port-tooltip-signal">{getSignalTypeLabel(port)}</div>
{/if}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
{#if port.mgmt_only}
<div class="port-tooltip-badge mgmt">Management Only</div>
{/if}
Expand Down Expand Up @@ -149,7 +165,8 @@
font-size: var(--font-size-xs);
}

.port-tooltip-direction {
.port-tooltip-direction,
.port-tooltip-signal {
color: var(--colour-text-muted-inverse, rgba(255, 255, 255, 0.7));
font-size: var(--font-size-xs);
}
Expand Down
22 changes: 22 additions & 0 deletions src/lib/schemas/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,25 @@ export const InterfacePositionSchema = z.enum(["front", "rear"]);
*/
export const PortDirectionSchema = z.enum(["input", "output", "bidirectional"]);

/**
* Signal type enum - what a port carries, independent of the connector
* (spike #1927; #1935). The connector (InterfaceType) describes the plug;
* the signal type describes what flows through it (an XLR can carry mic,
* line, or AES3). Deliberately starts small; the taxonomy can grow.
*/
export const SignalTypeSchema = z.enum([
"analog-audio-mic",
"analog-audio-line",
"analog-audio-speaker",
"digital-audio-aes3",
"digital-audio-dante",
"digital-audio-avb",
"digital-video-hdmi",
"digital-video-sdi",
"clock-word",
"control-midi",
]);

// ============================================================================
// Container Slot Schemas (v0.6.0)
// ============================================================================
Expand Down Expand Up @@ -302,6 +321,7 @@ export const InterfaceTemplateSchema = z
poe_mode: PoEModeSchema.optional(),
poe_type: PoETypeSchema.optional(),
direction: PortDirectionSchema.optional(),
signal_type: SignalTypeSchema.optional(),
})
.passthrough();

Expand Down Expand Up @@ -381,6 +401,7 @@ export const PlacedPortSchema = z
type: InterfaceTypeSchema,
label: z.string().max(64).optional(),
direction: PortDirectionSchema.optional(),
signal_type: SignalTypeSchema.optional(),
})
.passthrough();

Expand Down Expand Up @@ -1110,6 +1131,7 @@ export type PoEType = z.infer<typeof PoETypeSchema>;
export type PoEMode = z.infer<typeof PoEModeSchema>;
export type InterfacePosition = z.infer<typeof InterfacePositionSchema>;
export type PortDirection = z.infer<typeof PortDirectionSchema>;
export type SignalType = z.infer<typeof SignalTypeSchema>;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
export type InterfaceTemplate = z.infer<typeof InterfaceTemplateSchema>;
export type PowerPort = z.infer<typeof PowerPortSchema>;
export type PowerOutlet = z.infer<typeof PowerOutletSchema>;
Expand Down
21 changes: 21 additions & 0 deletions src/lib/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,23 @@ export type InterfacePosition = "front" | "rear";
*/
export type PortDirection = "input" | "output" | "bidirectional";

/**
* Signal type carried by a port, independent of the physical connector.
* The connector (InterfaceType) describes the plug; the signal type describes
* what flows through it (e.g. an XLR can carry mic, line, or AES3).
*/
export type SignalType =
| "analog-audio-mic"
| "analog-audio-line"
| "analog-audio-speaker"
| "digital-audio-aes3"
| "digital-audio-dante"
| "digital-audio-avb"
| "digital-video-hdmi"
| "digital-video-sdi"
| "clock-word"
| "control-midi";

// =============================================================================
// Component Types (NetBox-compatible, schema-only)
// =============================================================================
Expand Down Expand Up @@ -243,6 +260,8 @@ export interface InterfaceTemplate {
poe_type?: PoEType;
/** Signal direction. Provides the default used when the placed port omits its own. */
direction?: PortDirection;
/** Signal carried by this port (explicit; inferred from type and direction when unset) */
signal_type?: SignalType;
}

/**
Expand Down Expand Up @@ -364,6 +383,8 @@ export interface PlacedPort {
label?: string;
/** Signal direction override; falls back to the InterfaceTemplate default when unset */
direction?: PortDirection;
/** Signal override; falls back to the template value, then inference, when unset */
signal_type?: SignalType;
}

/**
Expand Down
75 changes: 74 additions & 1 deletion src/lib/utils/port-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,13 @@
* Functions for port instantiation when devices are placed
*/

import type { DeviceType, PlacedPort, PortDirection } from "$lib/types";
import type {
DeviceType,
InterfaceType,
PlacedPort,
PortDirection,
SignalType,
} from "$lib/types";
import { generateId } from "$lib/utils/device";

export type PortCategory = "network" | "power" | "console" | "av";
Expand Down Expand Up @@ -106,6 +112,73 @@ export function inferDirection(
return "bidirectional";
}

/**
* Human-readable signal names, one per SignalType value. Shared source for
* every surface that labels signals, so a label change lands everywhere.
*/
export const SIGNAL_LABELS: Record<SignalType, string> = {
"analog-audio-mic": "Mic level",
"analog-audio-line": "Line level",
"analog-audio-speaker": "Speaker level",
"digital-audio-aes3": "AES3",
"digital-audio-dante": "Dante",
"digital-audio-avb": "AVB",
"digital-video-hdmi": "HDMI",
"digital-video-sdi": "SDI",
"clock-word": "Word clock",
"control-midi": "MIDI",
};

/**
* Label for a signal type, falling back to the raw slug for forward
* compatibility if a new value is not yet in SIGNAL_LABELS.
*/
export function getSignalLabel(signal: SignalType): string {
return SIGNAL_LABELS[signal] ?? signal;
}

/**
* Infer the signal a connector carries from its type (and, for XLR, its
* direction: mic level into an input, line level out of anything else).
* Returns undefined when the connector implies no distinct signal to label:
* network types (ethernet is the connector, not a separate signal), connectors
* whose signal has no SignalTypeSchema value yet (DMX, ADAT), and genuinely
* ambiguous ones. Device authors set signal_type explicitly for those.
*/
export function inferSignalType(
type: InterfaceType,
direction?: PortDirection,
): SignalType | undefined {
switch (type) {
case "xlr-3":
return direction === "input" ? "analog-audio-mic" : "analog-audio-line";
case "trs-1-4":
case "ts-1-4":
case "rca":
case "db25-audio":
case "phoenix":
return "analog-audio-line";
case "speakon":
return "analog-audio-speaker";
case "aes3":
return "digital-audio-aes3";
case "dante":
return "digital-audio-dante";
case "avb":
return "digital-audio-avb";
case "hdmi":
return "digital-video-hdmi";
case "sdi-bnc":
return "digital-video-sdi";
case "bnc":
return "clock-word";
case "midi-din":
return "control-midi";
default:
return undefined;
}
}

/**
* Instantiate ports from a DeviceType's interface templates
* Creates PlacedPort instances with stable UUIDs for each interface
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"allowList": [
{
"pathPattern": "\\.position$",
"reason": "rail position 3 is converted to internal 1/6U units on load; no coincidental duplicate elsewhere in this fixture unlike the sibling AV fixtures"
}
]
}
53 changes: 53 additions & 0 deletions src/tests/fixtures/upgrade-corpus/v26.7.0-signal-type.rackula.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
# yaml-language-server: $schema=https://count.racku.la/schemas/rackula-layout.schema.json
metadata:
id: 3f8a1c2d-9e4b-4c7a-8d1f-6b2e9a5c3d71
name: Signal Type Demo Rack
schema_version: "1.1"
version: "26.7.0"
name: Signal Type Demo Rack
racks:
- id: rack-signal
name: Signal Demo Rack
height: 8
width: 19
desc_units: false
show_rear: true
form_factor: 4-post-cabinet
starting_unit: 1
position: 0
devices:
- id: dev-signal-demo
device_type: signal-demo-device
position: 3
face: front
ports:
- id: port-mic-in
template_name: Mic In
template_index: 0
type: xlr-3
direction: input
- id: port-aes-out
template_name: AES Out
template_index: 1
type: xlr-3
direction: output
signal_type: digital-audio-aes3
device_types:
- slug: signal-demo-device
manufacturer: Acme Audio
model: Signal Demo 1U
u_height: 1
colour: "#8BE9FD"
category: av-media
interfaces:
- name: Mic In
type: xlr-3
direction: input
signal_type: analog-audio-mic
- name: AES Out
type: xlr-3
direction: output
signal_type: digital-audio-aes3
settings:
display_mode: label
show_labels_on_images: false
79 changes: 79 additions & 0 deletions src/tests/signal-type.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
import { describe, expect, it } from "vitest";
import {
InterfaceTemplateSchema,
PlacedPortSchema,
SignalTypeSchema,
} from "$lib/schemas";
import { getSignalLabel, inferSignalType } from "$lib/utils/port-utils";
import { createTestInterfaceTemplate, createTestPlacedPort } from "./factories";
import type { SignalType } from "$lib/types";

describe("inferSignalType", () => {
it("infers mic level for XLR inputs and line level otherwise", () => {
expect(inferSignalType("xlr-3", "input")).toBe("analog-audio-mic");
expect(inferSignalType("xlr-3", "output")).toBe("analog-audio-line");
expect(inferSignalType("xlr-3")).toBe("analog-audio-line");
});

it("maps analog connectors to line level and speakon to speaker level", () => {
expect(inferSignalType("trs-1-4")).toBe("analog-audio-line");
expect(inferSignalType("ts-1-4")).toBe("analog-audio-line");
expect(inferSignalType("rca")).toBe("analog-audio-line");
expect(inferSignalType("db25-audio")).toBe("analog-audio-line");
expect(inferSignalType("phoenix")).toBe("analog-audio-line");
expect(inferSignalType("speakon")).toBe("analog-audio-speaker");
});

it("maps digital audio, video, clock, and control connectors", () => {
expect(inferSignalType("aes3")).toBe("digital-audio-aes3");
expect(inferSignalType("dante")).toBe("digital-audio-dante");
expect(inferSignalType("avb")).toBe("digital-audio-avb");
expect(inferSignalType("hdmi")).toBe("digital-video-hdmi");
expect(inferSignalType("sdi-bnc")).toBe("digital-video-sdi");
expect(inferSignalType("bnc")).toBe("clock-word");
expect(inferSignalType("midi-din")).toBe("control-midi");
});

it("returns undefined when the connector implies no distinct signal", () => {
expect(inferSignalType("1000base-t")).toBeUndefined();
expect(inferSignalType("console")).toBeUndefined();
expect(inferSignalType("dmx-xlr")).toBeUndefined();
expect(inferSignalType("adat-optical")).toBeUndefined();
expect(inferSignalType("usb-c")).toBeUndefined();
});
});

describe("getSignalLabel", () => {
it("labels known signals and falls back to the raw slug", () => {
expect(getSignalLabel("analog-audio-mic")).toBe("Mic level");
expect(getSignalLabel("clock-word")).toBe("Word clock");
expect(getSignalLabel("future-signal" as SignalType)).toBe("future-signal");
});
});

describe("signal_type schema fields", () => {
it("accepts signal_type on interface templates and placed ports", () => {
const template = InterfaceTemplateSchema.parse(
createTestInterfaceTemplate({
type: "xlr-3",
signal_type: "digital-audio-aes3",
}),
);
expect(template.signal_type).toBe("digital-audio-aes3");

const port = PlacedPortSchema.parse(
createTestPlacedPort({ type: "xlr-3", signal_type: "analog-audio-mic" }),
);
expect(port.signal_type).toBe("analog-audio-mic");
});

it("rejects unknown signal_type values", () => {
expect(SignalTypeSchema.safeParse("laser-show").success).toBe(false);
expect(
InterfaceTemplateSchema.safeParse({
...createTestInterfaceTemplate(),
signal_type: "laser-show",
}).success,
).toBe(false);
});
});
Loading