feat: add signal_type field (optional, inferred) to InterfaceTemplate and PlacedPort - #3120
feat: add signal_type field (optional, inferred) to InterfaceTemplate and PlacedPort#3120githappens wants to merge 3 commits into
Conversation
🤖 CodeAnt AI — Review Status
Updated in place by CodeAnt AI · last 5 reviews |
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughAdds optional signal classifications for interface templates and placed ports, inference and labeling utilities, schema definitions, layout schema support, tooltip rendering, tests, and upgrade fixtures. ChangesSignal Type Feature
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant PortTooltip
participant PlacedPort
participant port_utils
PortTooltip->>PlacedPort: Read signal_type, direction, and connector type
PortTooltip->>port_utils: Infer signal type when needed
port_utils-->>PortTooltip: Return signal type or no result
PortTooltip->>port_utils: Convert signal type to label
port_utils-->>PortTooltip: Return display label
PortTooltip-->>PlacedPort: Render optional signal row
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
codeant-ai
Bot
added
the
size:L
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/components/PortTooltip.svelte`:
- Around line 110-112: Update the signal label rendering in PortTooltip to
compute getSignalTypeLabel(port) once using a Svelte {`@const`} block, then reuse
that local value for both the conditional check and displayed text.
In `@src/lib/schemas/index.ts`:
- Line 1224: Remove the duplicated exported SignalType inferred type from
SignalTypeSchema in the schemas module, retaining the canonical SignalType
definition from the types module. Update any local references or exports as
needed so consumers continue importing the canonical type without changing
SignalTypeSchema.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1447ec1d-73bf-4843-8e47-2c02056f5910
📒 Files selected for processing (8)
src/lib/components/PortTooltip.sveltesrc/lib/schemas/index.tssrc/lib/types/index.tssrc/lib/utils/port-utils.tssrc/tests/fixtures/upgrade-corpus/v26.7.0-signal-type.expected.jsonsrc/tests/fixtures/upgrade-corpus/v26.7.0-signal-type.rackula.yamlsrc/tests/signal-type.test.tsstatic/schemas/rackula-layout.schema.json
… placed ports Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Bence Kovács <23636204+githappens@users.noreply.github.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Bence Kovács <23636204+githappens@users.noreply.github.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Bence Kovács <23636204+githappens@users.noreply.github.com>
6f124c0 to
4717a74
Compare
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| 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; |
There was a problem hiding this comment.
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.
(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 fixThere was a problem hiding this comment.
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.
There was a problem hiding this comment.
✅ 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.sveltesrc/lib/components/PortIndicators.sveltesrc/lib/stores/portTooltip.svelte.ts
💡 To manage or update this instruction, visit: CodeAnt AI Settings
1 participant
User description
Closes #1935.
Adds the 10-value SignalTypeSchema from the issue AC, optional signal_type on
InterfaceTemplate and PlacedPort (a placed port's value overrides its template;
both are resolved at read time, mirroring how direction works), an
inferSignalType(type, direction) utility beside inferDirection, and a signal
row in PortTooltip.
Inference covers the connectors whose signal is unambiguous (XLR resolves
mic-vs-line via direction); DMX, ADAT, and other connectors without a taxonomy
value yet return undefined rather than guessing.
Schema changes are additive and backward compatible. SCHEMA_VERSION is left at
1.1: it was introduced earlier this release cycle and has not shipped yet (the
latest tag is still on 1.0), so per the same-release convention this optional,
additive field rides on 1.1 rather than triggering a second bump. A
current-format upgrade-corpus fixture (v26.7.0-signal-type, schema_version 1.1)
exercises the new fields, including an explicit AES3-over-XLR override beating
inference; its sidecar allowlists only the pre-existing rail-position
conversion. The published JSON schema is regenerated.
Not in scope, per the epic split: signal_type on Connection, compatibility
warnings (#1936), and CSV export (#1940).
CodeAnt-AI Description
Show signal type on ports and support explicit signal values
What Changed
Impact
✅ Clearer port details✅ Less manual signal labeling✅ Fewer wrong signal assumptions💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.