feat: add EtherCAT SDO/CoE support and device configuration UI

[marconetsf]

Summary

• ESI Repository & Parsing: Upload, parse and persist EtherCAT ESI XML files with lightweight summaries (v2 index) and on-demand full device loading via fast-xml-parser
• Device Configuration UI: Tabbed layout (Global Settings, Diagnostics, Devices) with full slave config (startup checks, addressing, timeouts, watchdog, distributed clocks)
• Channel Mapping: Auto-generated IEC 61131-3 addresses (%IX, %QW, %QD, etc.) with editable aliases and global address uniqueness
• SDO/CoE Object Dictionary: Parse CoE dictionary from ESI, display parameters grouped by parent object with collapsible sections, configurable startup SDO entries with float/double support
• Runtime Integration: Generate ethercat.json during compilation with full slave configs, runtime status monitoring panel, plugin-command API for device scanning
• EtherCAT Master Settings: Network interface selection, cycle time, watchdog timeout cycles configuration

Test plan

• Upload ESI XML files → repository table shows devices with expand/collapse
• Add device from repository or scan → appears in configured devices list
• Expand device → Configuration tab shows all settings (startup checks, timeouts, watchdog, DC)
• Channel Mappings tab shows auto-generated IEC addresses, editable aliases
• SDO Parameters tab shows CoE objects grouped by parent, configurable startup entries
• Diagnostics tab shows runtime EtherCAT status when connected
• Global Settings tab allows master config (interface, cycle time, watchdog)
• Compile project → conf/ethercat.json generated with correct structure
• Direction filter and search work in channel mapping table
• npm run start:dev builds without errors

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Full EtherCAT workflow: dedicated EtherCAT editor (Global Settings, Diagnostics, Devices), runtime diagnostics & scanning, device browser, importable ESI devices, and rich device configuration UIs (SDO/PDO, channel mappings, defaults).
  • ESI repository: upload/parse/migrate ESI XMLs, parse progress, repository table, and device enrichment/import.

• Improvements

  • EtherCAT selectable in device creation UI.
  • IO point generation now includes EtherCAT mappings.
  • Build now emits EtherCAT runtime config when applicable.

• Chores

  • Added XML parsing dependency and updated dev prestart script.

• Documentation

  • Added Portuguese "Architecture Survey — Autonomy Logic" guidance.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Adds end-to-end EtherCAT support: ESI parsing/persistence and IPC, EtherCAT discovery/runtime IPC, renderer EtherCAT editor and UI (repository, diagnostics, devices), new types/schemas, utilities to parse/match/enrich/generate runtime config, and compiler integration that emits conf/ethercat.json during compile/upload.

Changes

Cohort / File(s)	Summary
Docs & Build CLAUDE.md, configs/webpack/webpack.config.renderer.dev.ts, package.json	Added architecture survey instructions to CLAUDE.md; set ESLint plugin cache: false; updated prestart to remove tsbuildinfo; added fast-xml-parser dependency.
Main IPC & Compiler src/main/modules/ipc/main.ts, src/main/modules/ipc/renderer.ts, src/main/modules/compiler/compiler-module.ts	New IPC handlers for EtherCAT discovery/runtime and ESI repo/file ops; ESIService integration in main; added runtime API POST helper with token-refresh logic; wired EtherCAT config generation into compile/upload to write conf/ethercat.json.
ESI Service (main) src/main/services/esi-service/index.ts, src/main/services/esi-service/esi-parser-main.ts	New disk-backed ESIService with v1→v2 index migration, serialized writes, XML CRUD, and fast-xml-parser based light/full ESI parsers.
Types & Schemas src/types/ethercat/..., src/types/PLC/open-plc.ts	Added comprehensive EtherCAT/ESI types and Zod schemas; extended PLCRemoteDeviceSchema with optional ethercatConfig; exported EtherCAT-related types.
Renderer — EtherCAT editor & UI src/renderer/components/_features/.../ethercat/index.tsx, .../devices-tab.tsx, .../diagnostics-tab.tsx, .../global-settings-tab.tsx	New top-level EtherCATEditor and tabs: Global Settings, Diagnostics, Devices; runtime/repository integration, scanning flows, migration and store persistence.
Renderer — Repository & Upload .../esi-upload.tsx, .../esi-repository.tsx, .../esi-repository-table.tsx, .../esi-parse-progress.tsx	ESI upload UI with sequential parse+save, parse progress UI, repository table and clear/remove wired to IPC.
Renderer — Device config UI .../configured-devices.tsx, .../configured-device-row.tsx, .../device-detail-panel.tsx, .../sdo-parameters-table.tsx, .../channel-mapping-table.tsx	Device list/detail UI: load full ESI, auto-enrich CoE defaults, alias editing, SDO startup table and channel mapping management.
Renderer — Discovery & Scan UI .../device-scan-table.tsx, .../discovered-device-table.tsx, .../interface-selector.tsx, .../runtime-status-panel.tsx	Components for interface selection, scanning control, discovered devices listing, and runtime/plugin status with per-slave details.
Renderer Integrations & Store src/renderer/screens/workspace-screen.tsx, src/renderer/hooks/use-store-selectors.ts, src/renderer/store/slices/project/slice.ts, src/renderer/store/slices/project/types.ts	Conditional EtherCAT editor routing in workspace, included EtherCAT IO-points in selectors, added updateEthercatConfig action, and included EtherCAT IEC locations in used-address reservations; Zod action schema updated.
Utilities & Generators src/utils/ethercat/...	New ESI parsing utilities, pdo→channels, device matching, enrichment, SDO defaults, default slave config, and generateEthercatConfig(remoteDevices) to produce runtime JSON.
New Types Module src/types/ethercat/esi-types.ts, src/types/ethercat/index.ts	New ESI/EtherCAT declarations and IPC payload types used across main/renderer/services.

Sequence Diagram(s)

sequenceDiagram
    participant User as User (UI)
    participant Renderer as Renderer (Browser)
    participant Main as Main Process
    participant Runtime as Runtime API
    participant FS as File System

    User->>Renderer: Click "Scan" (ip, jwt, iface)
    Renderer->>Main: invoke "ethercat:scan"
    Main->>Runtime: POST /api/discovery/scan
    Runtime-->>Main: scan results
    Main-->>Renderer: scan result { success, data }

    User->>Renderer: Add selected device -> esiLoadDeviceFull(projectPath, id, idx)
    Renderer->>Main: invoke "esi:load-device-full"
    Main->>FS: read devices/esi/<id>.xml
    FS-->>Main: xml content
    Main->>Main: parseESIDeviceFull(xml, idx)
    Main-->>Renderer: parsed ESIDevice

    User->>Renderer: Save device config
    Renderer->>Store: dispatch updateEthercatConfig(...)
    Renderer->>Main: (on compile) invoke compile -> generateEthercatConfig(remoteDevices)
    Main->>FS: write conf/ethercat.json
    FS-->>Main: ack
    Main-->>Renderer: compile/upload logs

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

• feat: Add nested structures support for OPC-UA #547 — Adds runtime config generation hooks in CompilerModule (OPC‑UA); likely overlaps with this PR's compile-time config generation integration.
• Debugger implementation #367 — Overlaps IPC and compiler modifications; potential collisions with added IPC handlers and compile flow.
• feat: add remote device IO aliases to variables table location dropdown #580 — Modifies useRemoteDeviceIOPoints; directly related to EtherCAT IO-point additions in this PR.

Suggested labels

feature

Suggested reviewers

• vmleroy
• JoaoGSP
• thiagoralves

"🐰 I hopped through XML and code with delight,
mapping PDOs by day and SDOs by night.
Devices found and configs shaped with care,
EtherCAT now sings — I left a carrot there! 🥕"

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main objective of the changeset: adding EtherCAT SDO/CoE support and a device configuration UI, which are the central features introduced across the multiple new files and modules.
Description check	✅ Passed	The description includes a comprehensive Summary section detailing all major features, a detailed Test plan with most items checked, and references Claude Code as the generation tool. However, it lacks explicit references to specific issue/Jira tickets and does not include the full DOD checklist items requested by the template.
Docstring Coverage	✅ Passed	Docstring coverage is 90.32% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/ethercat-sdo-coe-support

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 16

🧹 Nitpick comments (7)

src/main/services/esi-service/index.ts (1)

86-100: Silent error suppression in loadRepositoryIndex.

Non-ENOENT errors (e.g., JSON parse errors, permission issues) are logged but return null, making them indistinguishable from a missing file. Consider returning an error state or throwing to allow callers to handle corrupted index files differently.

Suggested improvement

   async loadRepositoryIndex(projectPath: string): Promise<ESIRepositoryIndex | null> {
+  async loadRepositoryIndex(
+    projectPath: string
+  ): Promise<{ index: ESIRepositoryIndex | null; error?: string }> {
     const repoPath = this.getRepositoryPath(projectPath)
 
     try {
       const content = await promises.readFile(repoPath, 'utf-8')
       const index = JSON.parse(content) as ESIRepositoryIndex
-      return index
+      return { index }
     } catch (error) {
       if ((error as NodeJS.ErrnoException).code === 'ENOENT') {
-        return null
+        return { index: null }
       }
       console.error('Error reading ESI repository index:', error)
-      return null
+      return { index: null, error: error instanceof Error ? error.message : 'Unknown error' }
     }
   }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/services/esi-service/index.ts` around lines 86 - 100, The
loadRepositoryIndex function currently treats all non-ENOENT errors as a missing
index by logging and returning null, which hides parse/permission errors; update
loadRepositoryIndex (and adjust callers if necessary) to rethrow non-ENOENT
errors instead of returning null so callers can distinguish a missing file from
a corrupted/unreadable index (retain the existing null return for ENOENT);
reference this.getRepositoryPath, the loadRepositoryIndex method and the
ESIRepositoryIndex type when making the change.

src/utils/ethercat/esi-parser.ts (1)

248-268: Minor: Redundant case for bitLen === 1.

The switch statement at line 254 handles case 1 which returns 'BOOL', but this case is already covered by line 249 (bitLen === 1). This is not a bug but creates slightly redundant code.

Optional simplification

   // Handle BIT types
   if (esiType.startsWith('BIT') || bitLen === 1) {
     return 'BOOL'
   }
 
   // Infer from bit length
   switch (bitLen) {
-    case 1:
-      return 'BOOL'
     case 8:
       return 'BYTE'

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/utils/ethercat/esi-parser.ts` around lines 248 - 268, The code checks
bitLen === 1 twice: once in the BIT-type guard (if (esiType.startsWith('BIT') ||
bitLen === 1) return 'BOOL') and again as case 1 in the switch; simplify by
removing the redundant check — either drop "|| bitLen === 1" from the if so the
switch handles bitLen cases, or remove the "case 1" branch from the switch and
keep the existing early return; update the logic in esiType/bitLen handling in
the function containing esiType and bitLen accordingly.

src/renderer/components/_features/[workspace]/editor/device/ethercat/components/configured-device-row.tsx (2)

57-711: Consider extracting shared configuration form logic.

The ConfiguredDeviceRow component duplicates significant portions of the configuration form from DeviceDetailPanel. Consider extracting the configuration sections (Startup Checks, Addressing, Timeouts, Watchdog, Distributed Clocks) into reusable components to reduce duplication and ensure consistency.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/configured-device-row.tsx
around lines 57 - 711, ConfiguredDeviceRow duplicates the entire configuration
form also present in DeviceDetailPanel; extract the shared UI/logic for "Startup
Checks", "Addressing", "Timeouts", "Watchdog" and "Distributed Clocks (DC)" into
a reusable component (e.g., DeviceConfigurationForm or SharedDeviceConfig) that
accepts the device config, an update callback (use the existing updateConfig
shape or onUpdateDevice), and helpers like parseNumericInput; move the
InputWithRef/Checkbox wiring and disabled/validation behavior into that
component and reuse it from both ConfiguredDeviceRow and DeviceDetailPanel,
keeping the existing function names/props (updateConfig, parseNumericInput,
config, InputWithRef, Checkbox) so callers simply pass config and a handler to
apply partial updates.

---

103-159: Dependency array includes properties that won't trigger re-runs due to ref guard.

Similar to DeviceDetailPanel, the useEffect includes dependencies like device.channelInfo, device.rxPdos, etc., but the fullDeviceLoadedRef.current guard prevents re-execution. The dependencies are technically correct for exhaustive-deps but could be misleading.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/configured-device-row.tsx
around lines 103 - 159, The effect's dependency list includes properties
(device.channelInfo, device.rxPdos, device.txPdos, etc.) that won’t cause
re-runs because fullDeviceLoadedRef.current short-circuits loadFullDevice;
simplify the deps to only the values that should actually trigger loading (e.g.,
isExpanded, projectPath, device.esiDeviceRef, device.channelMappings.length,
onUpdateChannelMappings, onEnrichDevice, externalAddresses) by removing the
individual device.* fields, or alternatively replace them with a single stable
identifier (like device.esiDeviceRef or a memoized device object) so useEffect,
fullDeviceLoadedRef, loadFullDevice and the dependency array reflect the real
triggers for reloading.

src/renderer/components/_features/[workspace]/editor/device/ethercat/components/device-detail-panel.tsx (1)

108-161: Effect dependencies may cause unnecessary re-runs.

The useEffect dependency array includes device.channelMappings.length, device.channelInfo, device.rxPdos, device.txPdos, and externalAddresses. Since fullDeviceLoadedRef.current guards against re-execution, consider reducing the dependency array to only essential triggers or splitting the effect.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/device-detail-panel.tsx
around lines 108 - 161, The effect is over-dependent causing needless re-runs;
keep only the true triggers for loading the full device and leave
enrichment/update handlers as stable deps: replace the dependency array in the
useEffect that defines loadFullDevice (the effect using
fullDeviceLoadedRef.current, setIsLoadingChannels,
window.bridge.esiLoadDeviceFull, pdoToChannels, generateDefaultChannelMappings,
onUpdateChannelMappings, onEnrichDevice, enrichDeviceData) with a minimal list:
[projectPath, device.esiDeviceRef, externalAddresses, onUpdateChannelMappings,
onEnrichDevice]; remove derived/volatile properties like
device.channelMappings.length, device.channelInfo, device.rxPdos, device.txPdos
from the deps (or move logic that reacts to those properties into a separate
effect if you need to respond to changes).

src/types/ethercat/esi-types.ts (1)

595-612: Inline type definition for device in ScannedDeviceMatch.

The device property duplicates the structure of EtherCATDevice from index.ts. Consider referencing the existing type or creating a shared base type to avoid duplication.

Suggested change

+import type { EtherCATDevice } from './index'
+
 export interface ScannedDeviceMatch {
   /** The scanned device from network discovery */
-  device: {
-    position: number
-    name: string
-    vendor_id: number
-    product_code: number
-    revision: number
-    serial_number: number
-    state: string
-    input_bytes: number
-    output_bytes: number
-  }
+  device: Pick<EtherCATDevice, 'position' | 'name' | 'vendor_id' | 'product_code' | 'revision' | 'serial_number' | 'state' | 'input_bytes' | 'output_bytes'>

Note: This would require careful handling of circular imports since index.ts re-exports from this file.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/types/ethercat/esi-types.ts` around lines 595 - 612, ScannedDeviceMatch
currently inlines a device object that duplicates the EtherCATDevice structure;
replace the inline device property with a reference to the shared type (e.g.,
EtherCATDevice or a new EtherCATDeviceBase) to avoid duplication—edit the
ScannedDeviceMatch interface to use that type for the device field and either
import EtherCATDevice from its module or extract a small shared type into a new
common types file to break potential circular imports; ensure ESIDeviceRef and
DeviceMatch usages remain unchanged and update exports so index.ts re-exports
the unified type without creating import cycles.

src/main/modules/ipc/renderer.ts (1)

438-455: Prefer a shared type alias over repeating the repository index item shape.

The inline nested items structure increases drift risk versus domain types already used in this file.

♻️ Suggested refactor

+type ESIRepositoryIndexPayload = {
+  version: number
+  items: ESIRepositoryItem[]
+}
+
   esiLoadRepositoryIndex: (
     projectPath: string,
   ): Promise<{
     success: boolean
-    data?: {
-      version: number
-      items: Array<{
-        id: string
-        filename: string
-        vendorId: string
-        vendorName: string
-        deviceCount: number
-        loadedAt: number
-        warnings?: string[]
-      }>
-    } | null
+    data?: ESIRepositoryIndexPayload | null
     error?: string
   }> => ipcRenderer.invoke('esi:load-repository-index', projectPath),

As per coding guidelines, src/**/*.{ts,tsx} should use strict TypeScript patterns and proper typing.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/modules/ipc/renderer.ts` around lines 438 - 455, Replace the inline
nested return type in esiLoadRepositoryIndex with a shared type alias: define a
RepositoryIndexItem (or reuse existing domain type if present) for the objects
in items and a RepositoryIndex (or RepositoryIndexResponse) for the overall data
shape, then update the esiLoadRepositoryIndex signature to reference these
aliases instead of repeating the inline structure; ensure the Promise return
type uses the new types (e.g., Promise<{ success: boolean; data?:
RepositoryIndex | null; error?: string }>) and import/export the types near
other IPC/domain types in this file so the shape is centralized and reusable.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/main/modules/ipc/main.ts`:
- Around line 596-600: The JSON body sent as postData for the ethercat scan
command omits scanRequest.timeout_ms, so pass timeout_ms through by adding
timeout_ms: scanRequest.timeout_ms into the params object when building postData
(the block that sets plugin: 'ethercat', command: 'scan', params: { interface:
scanRequest.interface }). Make the identical change in the other similar
postData creation (the second scan call around the other occurrence) so the
runtime receives and uses the UI-configured timeout.

In `@src/main/services/esi-service/index.ts`:
- Around line 396-442: parseAndSaveFile has a TOCTOU race between checking
existing filenames via loadRepositoryIndex and writing via saveXmlFile; make the
duplicate check-and-write atomic by either obtaining a project-level mutex/lock
around the sequence (check existingIndex, saveXmlFile, and
saveRepositoryIndexV2) or by using a write-idempotent approach (generate
deterministic item id from filename+project or allow duplicates and deduplicate
on read). Concretely, wrap the critical section in a lock (e.g.,
acquireLock(projectPath) before calling loadRepositoryIndex, saveXmlFile, and
saveRepositoryIndexV2, then releaseLock), or replace uuidv4() with a stable id
derived from filename and skip saving if the id already exists; update
parseAndSaveFile, loadRepositoryIndex, saveXmlFile, and saveRepositoryIndexV2
usage accordingly so the check and write are atomic.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/channel-mapping-table.tsx:
- Line 3: AliasCell seeds localAlias from the alias prop only on mount causing
stale input after parent updates; add an import for useEffect and inside
AliasCell add a useEffect that watches the alias prop and sets localAlias(alias)
whenever alias changes so the input stays in sync with prop updates and avoids
writing old values back on blur.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/configured-device-row.tsx:
- Around line 651-653: The call currently bypasses type checking by casting `{
coeObjects }` to `never`; instead pass a correctly typed ESIDevice (or a
properly constructed object matching ESIDevice) to enrichDeviceData and remove
the `as never` cast. Locate the button handler in configured-device-row.tsx
where onEnrichDevice is invoked and replace the unsafe cast with either the
actual device prop/variable (the same approach used in DeviceDetailPanel) or
build an ESIDevice object that includes coeObjects plus the required ESIDevice
fields before calling enrichDeviceData; ensure enrichDeviceData’s parameter type
is satisfied without using `as never`.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/configured-devices.tsx:
- Around line 55-56: selectedDeviceId is only cleared on local removals so when
the parent-provided devices list changes externally the selection can point to a
non-existent device; update the component to watch the devices array (via a
useEffect) and call setSelectedDeviceId(null) whenever the current
selectedDeviceId is not found in the updated devices (also ensure the same check
is applied where selection logic currently exists around lines referencing
selection handling, e.g., the code block that removes devices and where
selectedDeviceId is used) so the selection is always cleared when the selected
device is removed externally.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/device-browser-modal.tsx:
- Around line 193-200: The device rows use a plain interactive div which is not
keyboard-accessible; update the element representing each row (the element using
key={`${repoItem.id}-${deviceIndex}`} and className with isSelected) to be
keyboard-focusable and activate on Enter/Space by either converting the div to a
semantic <button> or adding role="button", tabIndex={0}, and an onKeyDown
handler that calls handleSelectDevice(repoItem.id, deviceIndex) when Enter or
Space is pressed; also ensure any selected state (isSelected) is exposed via
aria-pressed or aria-selected to assistive tech.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/device-detail-panel.tsx:
- Around line 614-616: The call enrichDeviceData({ coeObjects } as never)
unsafely casts a partial object to ESIDevice and causes runtime failures when
enrichDeviceData (and helpers persistPdos / deriveSlaveType) access rxPdo/txPdo;
fix by passing a valid ESIDevice or making enrichDeviceData accept
Partial<ESIDevice> and handle missing fields. Replace the cast in the onClick
handler with a properly constructed ESIDevice object containing the required
fields (at minimum the rxPdo/txPdo structures and any other non-optional
properties used by persistPdos/deriveSlaveType) or update enrichDeviceData (and
its callers) to accept Partial<ESIDevice> and add runtime guards/defaults for
rxPdo/txPdo before calling persistPdos/deriveSlaveType.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/esi-channels-table.tsx:
- Around line 8-9: The header "select all" handler must pass the IDs of the
currently filtered rows instead of only a boolean: change the prop signature
onChannelSelectAll from (selected: boolean) => void to (selected: boolean,
channelIds: string[]) => void and update every call site in this component
(where you compute filteredChannels) to call onChannelSelectAll(selected,
Array.from(filteredChannels.keys()) or the equivalent filteredChannels.map(c =>
c.id)). Also update any parent/component usages of onChannelSelectAll to accept
the new second parameter; ensure the checkbox state still derives from
filteredChannels but the action operates only on the passed IDs.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/esi-repository-table.tsx:
- Around line 111-116: The icon-only button using onToggleExpand and ArrowIcon
lacks an accessible name and does not expose expanded state; update the button
to include aria-expanded={isExpanded} and a clear aria-label that reflects the
state (e.g., "Expand row" vs "Collapse row") and, if the expandable panel has an
id, add aria-controls pointing to that id so screen readers can relate the
button to the content; ensure the expandable region component sets the matching
id.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/esi-repository.tsx:
- Around line 128-130: The upload error list uses error.filename alone as the
React list key which can collide for duplicate or empty names; update the key in
the uploadErrors.map call to use a stable unique identifier (for example combine
error.filename with the map index or use a unique error.id if available) so the
<div key=...> for each error row is guaranteed unique; modify the JSX in the
mapping that renders uploadErrors to use that combined/unique value instead of
just error.filename.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/esi-upload.tsx:
- Line 40: The filter for XML files only matches lowercase ".xml" so uppercase
extensions like "DEVICE.XML" get ignored; update the filter in the handler that
computes xmlFiles to perform a case-insensitive check (e.g., compare
file.name.toLowerCase().endsWith('.xml') or use a case-insensitive regex) so
filenames with any capitalization are accepted — locate the xmlFiles variable
and replace the current .endsWith('.xml') check with a case-insensitive
alternative.
- Around line 155-167: The drop-zone div uses onClick but lacks keyboard
semantics; add keyboard accessibility by making the div focusable and act like a
button: add tabIndex={0}, role="button", and an onKeyDown handler that invokes
handleClick when Enter or Space is pressed; also reflect the disabled state when
isProcessing by setting aria-disabled={isProcessing} and preserving
pointer-events and visual focus styles so keyboard users can trigger the file
picker the same way as mouse users (target the element with
onClick/onDragOver/onDragLeave/onDrop and the isProcessing prop/state).

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/sdo-parameters-table.tsx:
- Line 4: ValueCell is maintaining a stale localValue when upstream entry.value
(from sdoConfigurations) is updated (e.g., "Reset All to Defaults"); add a
useEffect inside the ValueCell component that watches entry.value and calls
setLocalValue(entry.value) to resync the input whenever entry.value changes,
ensuring the initial state is still derived from entry.value and avoiding stale
values on blur/submit.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/index.tsx:
- Around line 302-312: The effect watching isConnectedToRuntime is re-triggering
because checkServiceStatus and fetchInterfaces change identity each render; make
those callbacks stable (or call their logic inside the effect) so the effect
only depends on isConnectedToRuntime. Fix by memoizing checkServiceStatus and
fetchInterfaces with useCallback (ensuring their internal dependencies are
included) or move their implementations into the useEffect body (or expose
stable refs) and then change the dependency array to [isConnectedToRuntime] so
useEffect no longer loops; reference the existing useEffect and the
checkServiceStatus and fetchInterfaces functions to locate and update the code.

In `@src/utils/ethercat/sdo-config-defaults.ts`:
- Around line 50-57: The current code turns missing defaults into
empty-string/zero writes by using sub.defaultValue ?? '' (and similarly
obj.defaultValue ?? '') when building entries; change the logic in the SDO
defaults builder so you only add an entries.push for a parameter if a real
default exists (e.g., check if sub.defaultValue !== undefined/null before
constructing defaultValue and calling entries.push with index: obj.index,
subIndex: subIdx, value: defaultValue, defaultValue), and apply the same guard
to the other block that handles obj.defaultValue (lines ~67-73) so absent
defaults are skipped instead of emitted as startup writes.

---

Nitpick comments:
In `@src/main/modules/ipc/renderer.ts`:
- Around line 438-455: Replace the inline nested return type in
esiLoadRepositoryIndex with a shared type alias: define a RepositoryIndexItem
(or reuse existing domain type if present) for the objects in items and a
RepositoryIndex (or RepositoryIndexResponse) for the overall data shape, then
update the esiLoadRepositoryIndex signature to reference these aliases instead
of repeating the inline structure; ensure the Promise return type uses the new
types (e.g., Promise<{ success: boolean; data?: RepositoryIndex | null; error?:
string }>) and import/export the types near other IPC/domain types in this file
so the shape is centralized and reusable.

In `@src/main/services/esi-service/index.ts`:
- Around line 86-100: The loadRepositoryIndex function currently treats all
non-ENOENT errors as a missing index by logging and returning null, which hides
parse/permission errors; update loadRepositoryIndex (and adjust callers if
necessary) to rethrow non-ENOENT errors instead of returning null so callers can
distinguish a missing file from a corrupted/unreadable index (retain the
existing null return for ENOENT); reference this.getRepositoryPath, the
loadRepositoryIndex method and the ESIRepositoryIndex type when making the
change.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/configured-device-row.tsx:
- Around line 57-711: ConfiguredDeviceRow duplicates the entire configuration
form also present in DeviceDetailPanel; extract the shared UI/logic for "Startup
Checks", "Addressing", "Timeouts", "Watchdog" and "Distributed Clocks (DC)" into
a reusable component (e.g., DeviceConfigurationForm or SharedDeviceConfig) that
accepts the device config, an update callback (use the existing updateConfig
shape or onUpdateDevice), and helpers like parseNumericInput; move the
InputWithRef/Checkbox wiring and disabled/validation behavior into that
component and reuse it from both ConfiguredDeviceRow and DeviceDetailPanel,
keeping the existing function names/props (updateConfig, parseNumericInput,
config, InputWithRef, Checkbox) so callers simply pass config and a handler to
apply partial updates.
- Around line 103-159: The effect's dependency list includes properties
(device.channelInfo, device.rxPdos, device.txPdos, etc.) that won’t cause
re-runs because fullDeviceLoadedRef.current short-circuits loadFullDevice;
simplify the deps to only the values that should actually trigger loading (e.g.,
isExpanded, projectPath, device.esiDeviceRef, device.channelMappings.length,
onUpdateChannelMappings, onEnrichDevice, externalAddresses) by removing the
individual device.* fields, or alternatively replace them with a single stable
identifier (like device.esiDeviceRef or a memoized device object) so useEffect,
fullDeviceLoadedRef, loadFullDevice and the dependency array reflect the real
triggers for reloading.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/device-detail-panel.tsx:
- Around line 108-161: The effect is over-dependent causing needless re-runs;
keep only the true triggers for loading the full device and leave
enrichment/update handlers as stable deps: replace the dependency array in the
useEffect that defines loadFullDevice (the effect using
fullDeviceLoadedRef.current, setIsLoadingChannels,
window.bridge.esiLoadDeviceFull, pdoToChannels, generateDefaultChannelMappings,
onUpdateChannelMappings, onEnrichDevice, enrichDeviceData) with a minimal list:
[projectPath, device.esiDeviceRef, externalAddresses, onUpdateChannelMappings,
onEnrichDevice]; remove derived/volatile properties like
device.channelMappings.length, device.channelInfo, device.rxPdos, device.txPdos
from the deps (or move logic that reacts to those properties into a separate
effect if you need to respond to changes).

In `@src/types/ethercat/esi-types.ts`:
- Around line 595-612: ScannedDeviceMatch currently inlines a device object that
duplicates the EtherCATDevice structure; replace the inline device property with
a reference to the shared type (e.g., EtherCATDevice or a new
EtherCATDeviceBase) to avoid duplication—edit the ScannedDeviceMatch interface
to use that type for the device field and either import EtherCATDevice from its
module or extract a small shared type into a new common types file to break
potential circular imports; ensure ESIDeviceRef and DeviceMatch usages remain
unchanged and update exports so index.ts re-exports the unified type without
creating import cycles.

In `@src/utils/ethercat/esi-parser.ts`:
- Around line 248-268: The code checks bitLen === 1 twice: once in the BIT-type
guard (if (esiType.startsWith('BIT') || bitLen === 1) return 'BOOL') and again
as case 1 in the switch; simplify by removing the redundant check — either drop
"|| bitLen === 1" from the if so the switch handles bitLen cases, or remove the
"case 1" branch from the switch and keep the existing early return; update the
logic in esiType/bitLen handling in the function containing esiType and bitLen
accordingly.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: e34803c4-a8ab-43c1-958b-9fddf13efde9

📥 Commits

Reviewing files that changed from the base of the PR and between 8817969 and 23dcfc0.

⛔ Files ignored due to path filters (1)

• package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (42)

• CLAUDE.md
• configs/webpack/webpack.config.renderer.dev.ts
• package.json
• src/main/modules/compiler/compiler-module.ts
• src/main/modules/ipc/main.ts
• src/main/modules/ipc/renderer.ts
• src/main/services/esi-service/esi-parser-main.ts
• src/main/services/esi-service/index.ts
• src/renderer/components/_features/[workspace]/create-element/element-card/index.tsx
• src/renderer/components/_features/[workspace]/editor/device/ethercat/components/channel-mapping-table.tsx
• src/renderer/components/_features/[workspace]/editor/device/ethercat/components/configured-device-row.tsx
• src/renderer/components/_features/[workspace]/editor/device/ethercat/components/configured-devices.tsx
• src/renderer/components/_features/[workspace]/editor/device/ethercat/components/device-browser-modal.tsx
• src/renderer/components/_features/[workspace]/editor/device/ethercat/components/device-detail-panel.tsx
• src/renderer/components/_features/[workspace]/editor/device/ethercat/components/device-scan-table.tsx
• src/renderer/components/_features/[workspace]/editor/device/ethercat/components/devices-tab.tsx
• src/renderer/components/_features/[workspace]/editor/device/ethercat/components/diagnostics-tab.tsx
• src/renderer/components/_features/[workspace]/editor/device/ethercat/components/discovered-device-table.tsx
• src/renderer/components/_features/[workspace]/editor/device/ethercat/components/esi-channels-table.tsx
• src/renderer/components/_features/[workspace]/editor/device/ethercat/components/esi-device-info.tsx
• src/renderer/components/_features/[workspace]/editor/device/ethercat/components/esi-parse-progress.tsx
• src/renderer/components/_features/[workspace]/editor/device/ethercat/components/esi-repository-table.tsx
• src/renderer/components/_features/[workspace]/editor/device/ethercat/components/esi-repository.tsx
• src/renderer/components/_features/[workspace]/editor/device/ethercat/components/esi-upload.tsx
• src/renderer/components/_features/[workspace]/editor/device/ethercat/components/global-settings-tab.tsx
• src/renderer/components/_features/[workspace]/editor/device/ethercat/components/interface-selector.tsx
• src/renderer/components/_features/[workspace]/editor/device/ethercat/components/runtime-status-panel.tsx
• src/renderer/components/_features/[workspace]/editor/device/ethercat/components/sdo-parameters-table.tsx
• src/renderer/components/_features/[workspace]/editor/device/ethercat/index.tsx
• src/renderer/hooks/use-store-selectors.ts
• src/renderer/screens/workspace-screen.tsx
• src/renderer/store/slices/project/slice.ts
• src/renderer/store/slices/project/types.ts
• src/types/PLC/open-plc.ts
• src/types/ethercat/esi-types.ts
• src/types/ethercat/index.ts
• src/utils/ethercat/device-config-defaults.ts
• src/utils/ethercat/device-matcher.ts
• src/utils/ethercat/enrich-device-data.ts
• src/utils/ethercat/esi-parser.ts
• src/utils/ethercat/generate-ethercat-config.ts
• src/utils/ethercat/sdo-config-defaults.ts

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Add keyboard-accessible semantics to the upload drop zone.

Line 155 uses an interactive div (onClick) without keyboard activation/focus, so keyboard-only users can’t trigger file picking.

♿ Suggested fix

       <div
+        role='button'
+        tabIndex={0}
         onClick={handleClick}
+        onKeyDown={(e) => {
+          if (e.key === 'Enter' || e.key === ' ') {
+            e.preventDefault()
+            handleClick()
+          }
+        }}
         onDragOver={handleDragOver}
         onDragLeave={handleDragLeave}
         onDrop={handleDrop}

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	<div
	onClick={handleClick}
	onDragOver={handleDragOver}
	onDragLeave={handleDragLeave}
	onDrop={handleDrop}
	className={cn(
	'flex cursor-pointer flex-col items-center justify-center rounded-lg border-2 border-dashed p-6 transition-colors',
	isDragging
	? 'bg-brand/10 dark:bg-brand/20 border-brand'
	: 'border-neutral-300 hover:border-brand hover:bg-neutral-50 dark:border-neutral-700 dark:hover:bg-neutral-800/50',
	isProcessing && 'pointer-events-none opacity-50',
	)}
	>
	<div
	role='button'
	tabIndex={0}
	onClick={handleClick}
	onKeyDown={(e) => {
	if (e.key === 'Enter' || e.key === ' ') {
	e.preventDefault()
	handleClick()
	}
	}}
	onDragOver={handleDragOver}
	onDragLeave={handleDragLeave}
	onDrop={handleDrop}
	className={cn(
	'flex cursor-pointer flex-col items-center justify-center rounded-lg border-2 border-dashed p-6 transition-colors',
	isDragging
	? 'bg-brand/10 dark:bg-brand/20 border-brand'
	: 'border-neutral-300 hover:border-brand hover:bg-neutral-50 dark:border-neutral-700 dark:hover:bg-neutral-800/50',
	isProcessing && 'pointer-events-none opacity-50',
	)}
	>

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/esi-upload.tsx
around lines 155 - 167, The drop-zone div uses onClick but lacks keyboard
semantics; add keyboard accessibility by making the div focusable and act like a
button: add tabIndex={0}, role="button", and an onKeyDown handler that invokes
handleClick when Enter or Space is pressed; also reflect the disabled state when
isProcessing by setting aria-disabled={isProcessing} and preserving
pointer-events and visual focus styles so keyboard users can trigger the file
picker the same way as mouse users (target the element with
onClick/onDragOver/onDragLeave/onDrop and the isProcessing prop/state).

[coderabbitai]

Actionable comments posted: 13

♻️ Duplicate comments (1)

src/renderer/components/_features/[workspace]/editor/device/ethercat/components/device-detail-panel.tsx (1)

614-616: ⚠️ Potential issue | 🟠 Major

Replace the as never escape hatch here.

This handler only has coeObjects, but enrichDeviceData() also reads PDO data. The cast removes the compiler error and leaves a runtime crash path on click. Use the same extractDefaultSdoConfigurations(coeObjects) path that configured-device-row.tsx already uses instead of calling enrichDeviceData() with a partial object.

Suggested change

+import { extractDefaultSdoConfigurations } from '@root/utils/ethercat/sdo-config-defaults'
@@
-                <button
-                  onClick={() => onEnrichDevice(enrichDeviceData({ coeObjects } as never))}
+                <button
+                  onClick={() => onUpdateSdoConfigurations(extractDefaultSdoConfigurations(coeObjects))}

As per coding guidelines, src/**/*.{ts,tsx}: "Use strict TypeScript patterns, proper typing, and avoid any types".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/device-detail-panel.tsx
around lines 614 - 616, Replace the unsafe cast by passing a fully populated
object built with extractDefaultSdoConfigurations: change the onClick handler
that currently calls onEnrichDevice(enrichDeviceData({ coeObjects } as never))
to call
onEnrichDevice(enrichDeviceData(extractDefaultSdoConfigurations(coeObjects))).
This uses the same extractDefaultSdoConfigurations(coeObjects) path as
configured-device-row.tsx to supply PDO/SDO data and avoids the runtime crash
while keeping types correct for enrichDeviceData and onEnrichDevice.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/main/services/esi-service/index.ts`:
- Around line 393-395: The call to saveRepositoryIndexV2 currently ignores its
non-throwing result; update the caller(s) to inspect the returned object (e.g.,
const res = await this.saveRepositoryIndexV2(projectPath, items)), check
res.success, and handle failures by logging the error details and either
throwing or returning an error result so the migration/upload does not report
success; additionally, if saveRepositoryIndexV2 fails, ensure you clean up any
partially written artifacts (e.g., remove the newly created XML from
parseAndSaveFile) to avoid orphaned files.
- Around line 246-260: The current saveRepositoryItem implementation trusts the
caller-supplied snapshot (existingItems) and builds updatedItems locally, which
can overwrite concurrent changes; instead, within the same locked/transactional
code path where you call saveXmlFile and saveRepositoryIndex, re-load the
authoritative index from storage (e.g., call the existing index-read method) and
merge/replace the item there before persisting so you never base updates on
stale external state; update the logic in saveRepositoryItem (and the similar
code path used at the other occurrence) to read the current index inside the
lock, apply the replacement of the item by id, then call saveRepositoryIndex
with that fresh merged list.
- Around line 361-395: migrateRepositoryToV2 currently reads and skips
unreadable XML files outside any lock and then overwrites the v2 index, risking
partial migrations; wrap the whole migration in withIndexLock so the index is
locked for the duration, and change the inner loop (calls to loadXmlFile and
parseESILight) to abort on the first load/parse failure by returning { success:
false, error: <descriptive message> } (or rethrow) instead of logging and
continuing; ensure no call to saveRepositoryIndexV2 occurs if any item failed,
and surface the specific filename and error in the returned error message so
callers can handle the atomic failure.
- Around line 97-110: The loadRepositoryIndex function currently returns null on
any read/parse failure which hides corrupt repository.json files; change its
error handling so only ENOENT returns null and all other errors (including
JSON.parse failures) are rethrown or propagated. In practice update the catch in
loadRepositoryIndex to detect (error as NodeJS.ErrnoException).code === 'ENOENT'
and return null only in that case, otherwise rethrow the error so callers can
surface/fail on a corrupted repository index instead of treating it as missing.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/configured-device-row.tsx:
- Around line 194-213: The row's click handler is on a non-focusable <tr>
(onSelect) and the expand icon button (ArrowIcon) is icon-only with no
accessible name or aria-expanded; make the selection and expansion operable via
keyboard by moving the selection handler from the <tr> to a focusable control
(e.g., a button or element with tabIndex and role) so onSelect is invoked from a
focusable element, and update the expand control (the button that calls
onToggleExpand and renders ArrowIcon) to include an accessible label (aria-label
or visually hidden text) and aria-expanded reflecting isExpanded; ensure the
focusable selection control and the expand button are distinct and both
keyboard-focusable.
- Around line 646-650: The fallback rendering for the CoE dictionary should only
appear after channel load completes and must distinguish between load error vs
empty result: update the conditional logic in configured-device-row.tsx (within
the component that references isLoadingChannels, channelLoadError and
coeObjects) so the block currently guarded by {!isLoadingChannels &&
!device.sdoConfigurations && coeObjects && coeObjects.length > 0} stays for the
"has objects" case, and add explicit branches when !isLoadingChannels &&
channelLoadError to render the load error, and when !isLoadingChannels &&
coeObjects && coeObjects.length === 0 to render the "no CoE dictionary"
empty-array message; ensure both places mentioned in the review (the block
around isLoadingChannels/coObjects and the similar block at the other
occurrence) are updated similarly.
- Around line 110-168: The effect can start multiple overlapping loads of
loadFullDevice causing races when responses apply state (setChannels,
setCoeObjects, onUpdateChannelMappings, onEnrichDevice); introduce an in-flight
guard token/ref (e.g., loadTokenRef) and set it before calling await
window.bridge.esiLoadDeviceFull(), capture the token in the closure, and after
each await verify the token still matches (ignoring the response if it doesn't)
so late responses won't overwrite newer state; ensure you still set
fullDeviceLoadedRef.current and clear/advance the token in finally so future
loads can run.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/device-detail-panel.tsx:
- Around line 609-613: The current conditional around rendering CoE content uses
truthiness of coeObjects and doesn't account for load completion or errors;
update the conditions in the device detail panel to gate fallbacks on load
completion and error state by checking isLoadingChannels and channelLoadError
(e.g., only show empty/error UI when !isLoadingChannels and channelLoadError is
set), explicitly branch on coeObjects?.length === 0 to render the “no CoE
objects” empty state, and keep the positive branch for coeObjects?.length > 0
(while still skipping the whole block when device.sdoConfigurations is present).
- Around line 114-167: The effect's fullDeviceLoadedRef guard never resets when
the component receives a different device and the unsafe `as never` cast
bypasses typing; change the guard to track device identity (e.g. reset or
replace fullDeviceLoadedRef with a lastLoadedKey computed from
device.esiDeviceRef.repositoryItemId + device.esiDeviceRef.deviceIndex) so
loadFullDevice runs for new devices, and update the dependency list to include
that key; remove the unsafe cast in the onClick that calls
onEnrichDevice(enrichDeviceData(...)) and instead provide the correctly shaped
data (use the same pattern as configured-device-row.tsx — either call a helper
like extractDefaultSdoConfigurations(coeObjects) or construct the full device
shape before calling enrichDeviceData) so types are preserved and enrichment is
safe.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/sdo-parameters-table.tsx:
- Around line 68-83: handleBlur currently treats every non-BOOL SDO as a JS
Number and clamps via getDataTypeRange, which breaks text/custom values, allows
fractional input for integer types, and loses precision for 64-bit integers
(LINT/ULINT). Update handling to branch by entry.dataType
(SDOConfigurationEntry.dataType): keep BOOL as boolean; for integer types
enforce integer semantics (reject or round fractional input and validate with
integer regex or BigInt parsing for >2^53 values), for 64-bit integer types use
string/BigInt-aware parsing and preserve as string when out of safe JS number
range, for floating types use parseFloat/Number and clamp via getDataTypeRange,
and for string/custom types avoid numeric parsing entirely and pass localValue
through to onValueChange. Also ensure the rendered input type and validation
reflect the chosen branch (e.g., type="checkbox" for BOOL, text input for big
integers/strings, number/step for floats) and reference handleBlur,
getDataTypeRange, onValueChange, localValue, and
entry.{index,subIndex,dataType,bitLength} when making changes.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/index.tsx:
- Around line 357-402: handleAddSelectedFromScan is currently adding devices
from scan even if a device with the same physical position already exists in
configuredDevices; update the function to skip any scan match whose position is
already present in configuredDevices (or will be added in this batch) before
creating and pushing a new ConfiguredEtherCATDevice. In practice, check
configuredDevices.some(d => d.position === match.device.position) (and also
check newDevices for duplicates) and continue/skip when true so duplicates are
not appended, then proceed with enrichment and pushing only for positions not
already configured.
- Around line 269-305: The effect's async loadRepository can apply results for a
stale projectPath; fence or cancel the work by capturing the current projectPath
(or generating a unique loadId) at the start of loadRepository and verifying it
before calling setRepository, setRepositoryError, or setting
repositoryLoadedRef; alternatively use an AbortController-like ref (e.g.,
repositoryLoadTokenRef) that you update on each projectPath change and check
inside the async flow before applying results and after awaiting
esiLoadRepositoryLight/esiMigrateRepository to ignore responses for old projects
and clean up the token in the effect cleanup.
- Around line 206-212: The current refresh logic sets selectedInterface using
the previous value if present, which can leave selectedInterface referencing an
interface that no longer exists; in the result handling after calling
window.bridge.etherCATGetInterfaces, validate the previous selectedInterface
against the newly fetchedInterfaces: if fetchedInterfaces is empty, clear
selectedInterface; otherwise if the previous selectedInterface is absent from
fetchedInterfaces, setSelectedInterface to fetchedInterfaces[0].name; otherwise
keep the previous value. Use the existing state setters (setInterfaces,
setSelectedInterface) and the fetchedInterfaces array to perform this
revalidation so scanDevices never uses a stale selectedInterface.

---

Duplicate comments:
In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/device-detail-panel.tsx:
- Around line 614-616: Replace the unsafe cast by passing a fully populated
object built with extractDefaultSdoConfigurations: change the onClick handler
that currently calls onEnrichDevice(enrichDeviceData({ coeObjects } as never))
to call
onEnrichDevice(enrichDeviceData(extractDefaultSdoConfigurations(coeObjects))).
This uses the same extractDefaultSdoConfigurations(coeObjects) path as
configured-device-row.tsx to supply PDO/SDO data and avoids the runtime crash
while keeping types correct for enrichDeviceData and onEnrichDevice.

🪄 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: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 04b2c4f0-88d6-4161-85bc-1db51555fac1

📥 Commits

Reviewing files that changed from the base of the PR and between 23dcfc0 and f87367b.

📒 Files selected for processing (7)

• src/main/services/esi-service/index.ts
• src/renderer/components/_features/[workspace]/editor/device/ethercat/components/channel-mapping-table.tsx
• src/renderer/components/_features/[workspace]/editor/device/ethercat/components/configured-device-row.tsx
• src/renderer/components/_features/[workspace]/editor/device/ethercat/components/device-detail-panel.tsx
• src/renderer/components/_features/[workspace]/editor/device/ethercat/components/sdo-parameters-table.tsx
• src/renderer/components/_features/[workspace]/editor/device/ethercat/index.tsx
• src/types/PLC/open-plc.ts

🚧 Files skipped from review as they are similar to previous changes (2)

• src/renderer/components/_features/[workspace]/editor/device/ethercat/components/channel-mapping-table.tsx
• src/types/PLC/open-plc.ts

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Make migration atomic and fail on the first unreadable XML.

This method bypasses withIndexLock, and it silently skips items whose XML cannot be loaded or reparsed before rewriting the v2 index. A concurrent write—or a single bad file—can therefore collapse the migrated catalog to a partial repository.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/services/esi-service/index.ts` around lines 361 - 395,
migrateRepositoryToV2 currently reads and skips unreadable XML files outside any
lock and then overwrites the v2 index, risking partial migrations; wrap the
whole migration in withIndexLock so the index is locked for the duration, and
change the inner loop (calls to loadXmlFile and parseESILight) to abort on the
first load/parse failure by returning { success: false, error: <descriptive
message> } (or rethrow) instead of logging and continuing; ensure no call to
saveRepositoryIndexV2 occurs if any item failed, and surface the specific
filename and error in the returned error message so callers can handle the
atomic failure.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Only show the CoE empty state after a completed load.

This tab never renders channelLoadError, and the current conditions only distinguish coeObjects.length > 0 from !coeObjects. That makes failures fall through to the wrong empty message, and a successful load with coeObjects = [] renders nothing because [] is truthy. Gate the fallback on a completed load and handle coeObjects?.length === 0 explicitly.

Also applies to: 623-627

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/device-detail-panel.tsx
around lines 609 - 613, The current conditional around rendering CoE content
uses truthiness of coeObjects and doesn't account for load completion or errors;
update the conditions in the device detail panel to gate fallbacks on load
completion and error state by checking isLoadingChannels and channelLoadError
(e.g., only show empty/error UI when !isLoadingChannels and channelLoadError is
set), explicitly branch on coeObjects?.length === 0 to render the “no CoE
objects” empty state, and keep the positive branch for coeObjects?.length > 0
(while still skipping the whole block when device.sdoConfigurations is present).

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (5)

src/renderer/components/_features/[workspace]/editor/device/ethercat/components/esi-upload.tsx (1)

155-167: ⚠️ Potential issue | 🟡 Minor

Add keyboard accessibility to the drop zone.

The drop zone uses onClick but lacks keyboard semantics (tabIndex, role="button", onKeyDown), preventing keyboard-only users from triggering the file picker. This was flagged in a previous review but appears unresolved.

♿ Suggested fix for keyboard accessibility

       <div
+        role='button'
+        tabIndex={0}
         onClick={handleClick}
+        onKeyDown={(e) => {
+          if (e.key === 'Enter' || e.key === ' ') {
+            e.preventDefault()
+            handleClick()
+          }
+        }}
         onDragOver={handleDragOver}
         onDragLeave={handleDragLeave}
         onDrop={handleDrop}
+        aria-disabled={isProcessing}
         className={cn(

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/esi-upload.tsx
around lines 155 - 167, The drop zone div currently only supports pointer
interaction via onClick; add keyboard accessibility by making it focusable and
operable: add tabIndex={0} and role="button" to the same div, and implement an
onKeyDown handler that calls the existing handleClick when Enter or Space is
pressed (ensure to prevent default for Space). Keep existing props (onDragOver,
onDragLeave, onDrop) and respect isProcessing to disable keyboard activation
when pointer-events are none; reference the div currently using handleClick,
handleDragOver, handleDragLeave, handleDrop, isDragging, and isProcessing to
locate where to add tabIndex, role, and onKeyDown.

src/main/services/esi-service/index.ts (2)

481-485: ⚠️ Potential issue | 🟠 Major

Result of saveRepositoryIndexV2 is not checked.

If the index save fails after the XML is written (line 467), the function still returns { success: true, item }, leaving the XML file orphaned without an index entry. The past review comment flagging this issue appears unresolved.

🐛 Proposed fix to check save result

         // Append to v2 index
         const currentItems = await this.loadLightItemsFromIndex(projectPath)
-        await this.saveRepositoryIndexV2(projectPath, [...currentItems, item])
+        const indexSaveResult = await this.saveRepositoryIndexV2(projectPath, [...currentItems, item])
+        if (!indexSaveResult.success) {
+          // Clean up orphaned XML file
+          await this.deleteXmlFile(projectPath, itemId)
+          return { success: false, error: indexSaveResult.error ?? 'Failed to save repository index' }
+        }

         return { success: true, item }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/services/esi-service/index.ts` around lines 481 - 485, The code
currently writes XML then appends to v2 index without checking
saveRepositoryIndexV2's result, causing a success response even if the index
save fails; update the calling code (the block that calls
loadLightItemsFromIndex and saveRepositoryIndexV2) to capture the return value
or throw from saveRepositoryIndexV2, check whether it succeeded, and only return
{ success: true, item } when the save indicates success—if saveRepositoryIndexV2
returns a boolean or result object, handle the failure path by rolling back or
deleting the written XML (or returning { success: false, error }) and propagate
an error; reference the functions saveRepositoryIndexV2 and
loadLightItemsFromIndex to locate and modify the code.

---

404-423: ⚠️ Potential issue | 🟡 Minor

Migration silently drops items that fail to load or parse.

If an XML file cannot be loaded or parsed (lines 406-418), the error is logged but the item is excluded from the migrated v2 index. This can cause silent data loss if some XML files are corrupted or temporarily unreadable.

Consider failing the entire migration on the first error to avoid partial data loss, or at minimum returning the list of skipped items in the response so users are aware.

🛡️ Suggested fix to fail on first error

         for (const indexItem of index.items) {
-          try {
-            const xmlResult = await this.loadXmlFile(projectPath, indexItem.id)
-            if (xmlResult.success && xmlResult.content) {
-              const parseResult = parseESILight(xmlResult.content, indexItem.filename)
-              if (parseResult.success && parseResult.vendor && parseResult.devices) {
-                items.push({
-                  id: indexItem.id,
-                  filename: indexItem.filename,
-                  vendor: parseResult.vendor,
-                  devices: parseResult.devices,
-                  loadedAt: indexItem.loadedAt,
-                  warnings: parseResult.warnings || indexItem.warnings,
-                })
-              }
-            }
-          } catch (err) {
-            console.error(`Failed to migrate ESI file ${indexItem.filename}:`, err)
+          const xmlResult = await this.loadXmlFile(projectPath, indexItem.id)
+          if (!xmlResult.success || !xmlResult.content) {
+            return { success: false, error: `Failed to load XML for ${indexItem.filename}: ${xmlResult.error}` }
+          }
+          const parseResult = parseESILight(xmlResult.content, indexItem.filename)
+          if (!parseResult.success || !parseResult.vendor || !parseResult.devices) {
+            return { success: false, error: `Failed to parse ${indexItem.filename}: ${parseResult.error}` }
           }
+          items.push({
+            id: indexItem.id,
+            filename: indexItem.filename,
+            vendor: parseResult.vendor,
+            devices: parseResult.devices,
+            loadedAt: indexItem.loadedAt,
+            warnings: parseResult.warnings || indexItem.warnings,
+          })
         }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/services/esi-service/index.ts` around lines 404 - 423, The migration
loop over index.items currently swallows failures (console.error) when
loadXmlFile or parseESILight fail and silently omits items from the migrated v2
index; change the catch in that loop to abort the migration by rethrowing a
descriptive Error (include indexItem.id and indexItem.filename and the original
err) instead of logging, so callers of this method (the migration entrypoint)
will fail the migration on the first load/parse error; alternatively, if you
prefer non-fatal behavior, collect skipped item metadata into a skippedItems
array and return it alongside the migrated items, but do not keep the current
silent-continue behavior.

src/renderer/components/_features/[workspace]/editor/device/ethercat/components/device-detail-panel.tsx (2)

597-632: ⚠️ Potential issue | 🟠 Major

Show the real CoE load state in the Startup Parameters tab.

Line 628 still treats load failures as “No CoE Object Dictionary available” because channelLoadError is never rendered here. A successful load with coeObjects = [] also falls through both branches and renders nothing. Mirror the channel-mapping tab’s error branch here and make the empty state explicit with !channelLoadError && (!coeObjects || coeObjects.length === 0).

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/device-detail-panel.tsx
around lines 597 - 632, The Startup Parameters tab currently treats load
failures and empty loads the same and omits rendering when coeObjects === [], so
update the conditional branches around device.sdoConfigurations and coeObjects
in the device-detail-panel component: add a branch that renders the
channelLoadError (same as the channel-mapping tab) when channelLoadError is set,
change the "no CoE Object Dictionary" empty-state condition to require
!channelLoadError && (!coeObjects || coeObjects.length === 0), and ensure the
Auto-configure button branch still uses coeObjects && coeObjects.length > 0 and
calls onUpdateSdoConfigurations(extractDefaultSdoConfigurations(coeObjects));
this will surface load errors and explicitly show an empty-state when the load
succeeded but returned no objects.

---

112-168: ⚠️ Potential issue | 🟠 Major

Reload the full-device cache when the backing ESI reference changes.

Line 116 is still a one-shot guard. devices-tab.tsx:229-239 remounts this panel when effectiveDevice.id changes, but the same instance will skip reloading when projectPath or device.esiDeviceRef changes for that id, leaving stale channels/coeObjects in state. Because this effect also has no cleanup/token, a dependency change while esiLoadDeviceFull(...) is in flight can still let an older response run Lines 130-145 against newer props. Please verify by switching the same configured device to a different ESI entry or project and confirming the panel refreshes instead of reusing the previous load.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/device-detail-panel.tsx
around lines 112 - 168, The one-shot guard fullDeviceLoadedRef prevents
reloading when the backing ESI reference or project changes and allows stale
async responses to overwrite newer props; remove or reset that guard when
projectPath or device.esiDeviceRef changes and add a cancellation token/sequence
check around the async loadFullDevice (the call to
window.bridge.esiLoadDeviceFull) so only the latest response updates state.
Specifically, reset fullDeviceLoadedRef.current (or remove it) when projectPath
or device.esiDeviceRef changes, include a local requestId/abort flag checked
before calling setChannels, setCoeObjects, onUpdateChannelMappings, and
onEnrichDevice, and increment/flip that token on each effect run to ignore
previous in-flight responses.

🧹 Nitpick comments (1)

src/renderer/components/_features/[workspace]/editor/device/ethercat/components/esi-upload.tsx (1)

168-168: Consider making the accept attribute case-insensitive.

Some browsers treat the accept attribute case-sensitively, which may hide .XML files in the file picker. While processFiles correctly handles any case, adding both extensions improves the file picker UX.

💡 Suggested enhancement

-        <input ref={fileInputRef} type='file' accept='.xml' multiple onChange={handleFileSelect} className='hidden' />
+        <input ref={fileInputRef} type='file' accept='.xml,.XML' multiple onChange={handleFileSelect} className='hidden' />

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/esi-upload.tsx
at line 168, The file input's accept attribute is case-sensitive in some
browsers and currently only lists '.xml'; update the JSX input element (the one
using fileInputRef and onChange={handleFileSelect} in esi-upload.tsx) to include
both lowercase and uppercase extensions (e.g., accept='.xml,.XML') so the file
picker shows XML files regardless of casing; no changes needed to
processFiles/handleFileSelect behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/device-detail-panel.tsx:
- Around line 139-145: The current logic calls
onEnrichDevice(enrichDeviceData(result.device)) unconditionally when any
enrichment is needed, which overwrites device.sdoConfigurations (and slaveType)
even if they already exist as empty arrays; instead call
enrichDeviceData(result.device) into a local enriched variable and construct a
new device object that copies only missing fields: for example keep original
device.sdoConfigurations if it is defined (only take enriched.sdoConfigurations
when device.sdoConfigurations === undefined), and do the same for slaveType,
while still replacing channelInfo, rxPdos, txPdos when those are missing; then
pass that merged object to onEnrichDevice so you only backfill truly-missing
fields (referencing needsEnrichment, enrichDeviceData, onEnrichDevice,
device.sdoConfigurations, and slaveType).

---

Duplicate comments:
In `@src/main/services/esi-service/index.ts`:
- Around line 481-485: The code currently writes XML then appends to v2 index
without checking saveRepositoryIndexV2's result, causing a success response even
if the index save fails; update the calling code (the block that calls
loadLightItemsFromIndex and saveRepositoryIndexV2) to capture the return value
or throw from saveRepositoryIndexV2, check whether it succeeded, and only return
{ success: true, item } when the save indicates success—if saveRepositoryIndexV2
returns a boolean or result object, handle the failure path by rolling back or
deleting the written XML (or returning { success: false, error }) and propagate
an error; reference the functions saveRepositoryIndexV2 and
loadLightItemsFromIndex to locate and modify the code.
- Around line 404-423: The migration loop over index.items currently swallows
failures (console.error) when loadXmlFile or parseESILight fail and silently
omits items from the migrated v2 index; change the catch in that loop to abort
the migration by rethrowing a descriptive Error (include indexItem.id and
indexItem.filename and the original err) instead of logging, so callers of this
method (the migration entrypoint) will fail the migration on the first
load/parse error; alternatively, if you prefer non-fatal behavior, collect
skipped item metadata into a skippedItems array and return it alongside the
migrated items, but do not keep the current silent-continue behavior.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/device-detail-panel.tsx:
- Around line 597-632: The Startup Parameters tab currently treats load failures
and empty loads the same and omits rendering when coeObjects === [], so update
the conditional branches around device.sdoConfigurations and coeObjects in the
device-detail-panel component: add a branch that renders the channelLoadError
(same as the channel-mapping tab) when channelLoadError is set, change the "no
CoE Object Dictionary" empty-state condition to require !channelLoadError &&
(!coeObjects || coeObjects.length === 0), and ensure the Auto-configure button
branch still uses coeObjects && coeObjects.length > 0 and calls
onUpdateSdoConfigurations(extractDefaultSdoConfigurations(coeObjects)); this
will surface load errors and explicitly show an empty-state when the load
succeeded but returned no objects.
- Around line 112-168: The one-shot guard fullDeviceLoadedRef prevents reloading
when the backing ESI reference or project changes and allows stale async
responses to overwrite newer props; remove or reset that guard when projectPath
or device.esiDeviceRef changes and add a cancellation token/sequence check
around the async loadFullDevice (the call to window.bridge.esiLoadDeviceFull) so
only the latest response updates state. Specifically, reset
fullDeviceLoadedRef.current (or remove it) when projectPath or
device.esiDeviceRef changes, include a local requestId/abort flag checked before
calling setChannels, setCoeObjects, onUpdateChannelMappings, and onEnrichDevice,
and increment/flip that token on each effect run to ignore previous in-flight
responses.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/esi-upload.tsx:
- Around line 155-167: The drop zone div currently only supports pointer
interaction via onClick; add keyboard accessibility by making it focusable and
operable: add tabIndex={0} and role="button" to the same div, and implement an
onKeyDown handler that calls the existing handleClick when Enter or Space is
pressed (ensure to prevent default for Space). Keep existing props (onDragOver,
onDragLeave, onDrop) and respect isProcessing to disable keyboard activation
when pointer-events are none; reference the div currently using handleClick,
handleDragOver, handleDragLeave, handleDrop, isDragging, and isProcessing to
locate where to add tabIndex, role, and onKeyDown.

---

Nitpick comments:
In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/esi-upload.tsx:
- Line 168: The file input's accept attribute is case-sensitive in some browsers
and currently only lists '.xml'; update the JSX input element (the one using
fileInputRef and onChange={handleFileSelect} in esi-upload.tsx) to include both
lowercase and uppercase extensions (e.g., accept='.xml,.XML') so the file picker
shows XML files regardless of casing; no changes needed to
processFiles/handleFileSelect behavior.

🪄 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: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: fc4bfbbb-d8bb-4c1a-9535-399e904d100f

📥 Commits

Reviewing files that changed from the base of the PR and between f87367b and f6645fc.

📒 Files selected for processing (6)

• src/main/services/esi-service/index.ts
• src/renderer/components/_features/[workspace]/editor/device/ethercat/components/configured-devices.tsx
• src/renderer/components/_features/[workspace]/editor/device/ethercat/components/device-detail-panel.tsx
• src/renderer/components/_features/[workspace]/editor/device/ethercat/components/esi-upload.tsx
• src/renderer/components/_features/[workspace]/editor/device/ethercat/index.tsx
• src/utils/ethercat/sdo-config-defaults.ts

✅ Files skipped from review due to trivial changes (1)

• src/utils/ethercat/sdo-config-defaults.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/renderer/components/_features/[workspace]/editor/device/ethercat/components/configured-devices.tsx

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (5)

src/renderer/components/_features/[workspace]/editor/device/ethercat/components/configured-device-row.tsx (3)

684-688: ⚠️ Potential issue | 🟡 Minor

Same empty coeObjects array issue as device-detail-panel.tsx.

If coeObjects === [], neither the "auto-config" section (line 660 requires length > 0) nor the "no CoE" fallback (line 684 checks !coeObjects) renders. Apply the same fix: check !coeObjects || coeObjects.length === 0.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/configured-device-row.tsx
around lines 684 - 688, The conditional that shows the "No CoE Object
Dictionary" fallback incorrectly treats an empty coeObjects array as present;
update the JSX condition in the component (configured-device-row / the block
that currently checks !isLoadingChannels && !channelLoadError &&
!device.sdoConfigurations && !coeObjects) to instead check !coeObjects ||
coeObjects.length === 0 so an empty array triggers the fallback; mirror the same
check used by the "auto-config" section which expects coeObjects.length > 0.

---

107-175: ⚠️ Potential issue | 🟠 Major

Same fullDeviceLoadedRef issue as device-detail-panel.tsx.

The ref never resets when the component receives a different device. Additionally, if dependencies like externalAddresses change while a load is in-flight, a second load can race with the first. Use a device-identity key in the ref and consider an in-flight cancellation token.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/configured-device-row.tsx
around lines 107 - 175, fullDeviceLoadedRef currently never resets for a new
device and in-flight loads can race with updated dependencies; change
fullDeviceLoadedRef to store a device-identity key (e.g., combine
device.esiDeviceRef.repositoryItemId and device.esiDeviceRef.deviceIndex or a
device.id) and reset it when the identity changes, include that identity in the
effect deps, and add an in-flight cancellation token (AbortController or a local
"cancelled" flag checked after awaiting window.bridge.esiLoadDeviceFull in the
loadFullDevice function) so that if externalAddresses or the device identity
changes mid-load you bail out and don't apply stale setState or call
onEnrichDevice/onUpdateChannelMappings.

---

201-240: ⚠️ Potential issue | 🟠 Major

Accessibility: row selection and expand button lack keyboard support.

The onClick on <tr> (line 202) is not keyboard-accessible. The expand button (lines 209-220) lacks aria-label and aria-expanded. Screen reader users and keyboard navigators cannot interact with these controls effectively.

🛠️ Suggested fixes

       <tr
-        onClick={onSelect}
+        tabIndex={0}
+        role="row"
+        onClick={onSelect}
+        onKeyDown={(e) => {
+          if (e.key === 'Enter' || e.key === ' ') {
+            e.preventDefault()
+            onSelect()
+          }
+        }}
         className={cn(
           'cursor-pointer border-b border-neutral-200 dark:border-neutral-800',
           isSelected && 'bg-brand/10 dark:bg-brand/20',
         )}
       >
         <td className='px-2 py-2'>
           <button
+            aria-label={isExpanded ? 'Collapse device details' : 'Expand device details'}
+            aria-expanded={isExpanded}
             onClick={(e) => {
               e.stopPropagation()
               onToggleExpand()
             }}
             className='flex items-center justify-center'
           >

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/configured-device-row.tsx
around lines 201 - 240, The table row's click handler (onSelect) isn't
keyboard-accessible and the expand button (onToggleExpand / ArrowIcon) lacks
ARIA; make the row focusable and handle keyboard activation by adding tabIndex=0
to the <tr> and an onKeyDown that calls onSelect when Enter or Space is pressed
(mirror the existing onClick behavior), and add accessible attributes to the
expand button: include aria-expanded={isExpanded} and a descriptive aria-label
(e.g., `Expand ${device.name}`) while keeping e.stopPropagation() so the button
doesn't trigger row selection.

src/renderer/components/_features/[workspace]/editor/device/ethercat/components/device-detail-panel.tsx (2)

623-655: ⚠️ Potential issue | 🟡 Minor

Empty coeObjects array leaves the UI blank.

If the ESI load succeeds but returns coeObjects = [], the condition at line 627 (coeObjects.length > 0) fails and line 651 (!coeObjects) also fails since [] is truthy. The user sees nothing in the Startup Parameters section. Add an explicit branch for the empty-array case:

🛠️ Suggested fix

             {!isLoadingChannels &&
               !channelLoadError &&
               !device.sdoConfigurations &&
               coeObjects &&
               coeObjects.length > 0 && (
                 // ... auto-config button ...
               )}

+            {!isLoadingChannels &&
+              !channelLoadError &&
+              !device.sdoConfigurations &&
+              coeObjects &&
+              coeObjects.length === 0 && (
+                <p className='py-4 text-center text-sm text-neutral-500 dark:text-neutral-400'>
+                  No CoE Object Dictionary available for this device.
+                </p>
+              )}

             {!isLoadingChannels && !channelLoadError && !device.sdoConfigurations && !coeObjects && (
               <p className='py-4 text-center text-sm text-neutral-500 dark:text-neutral-400'>
                 No CoE Object Dictionary available for this device.
               </p>
             )}

Or simplify by changing line 651's condition to (!coeObjects || coeObjects.length === 0).

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/device-detail-panel.tsx
around lines 623 - 655, The UI currently shows nothing when ESI returns an empty
array because the first branch checks coeObjects.length > 0 and the "no CoE"
message checks !coeObjects (which is false for []). Update the component logic:
either add an explicit branch that checks Array.isArray(coeObjects) &&
coeObjects.length === 0 and render the "No CoE Object Dictionary available..."
message, or change the existing final condition that uses !coeObjects to instead
use (!coeObjects || coeObjects.length === 0) so the empty-array case is handled;
reference the coeObjects variable and the surrounding conditions
isLoadingChannels, channelLoadError, and device.sdoConfigurations when making
this change.

---

115-178: ⚠️ Potential issue | 🟠 Major

The fullDeviceLoadedRef guard doesn't reset when the device identity changes.

If this component receives a different device prop (different repositoryItemId or deviceIndex), the ref remains true and loadFullDevice never runs for the new device, leaving stale channel/CoE data displayed. Consider tracking device identity in the ref or resetting it based on device.esiDeviceRef:

🛠️ Suggested approach

-  const fullDeviceLoadedRef = useRef(false)
+  const lastLoadedKeyRef = useRef<string | null>(null)

   // Load full device data on mount
   useEffect(() => {
-    if (fullDeviceLoadedRef.current) return
+    const deviceKey = `${device.esiDeviceRef.repositoryItemId}-${device.esiDeviceRef.deviceIndex}`
+    if (lastLoadedKeyRef.current === deviceKey) return

     const loadFullDevice = async () => {
       // ... existing code ...
-          fullDeviceLoadedRef.current = true
+          lastLoadedKeyRef.current = deviceKey
       // ...
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/device-detail-panel.tsx
around lines 115 - 178, The guard fullDeviceLoadedRef prevents reload when the
device prop changes; compare the current device identity (e.g.,
device.esiDeviceRef.repositoryItemId + ':' + device.esiDeviceRef.deviceIndex) to
a stored lastDeviceIdRef inside the same useEffect and reset
fullDeviceLoadedRef.current = false when they differ before the early return, so
loadFullDevice runs for new devices; update lastDeviceIdRef with the new
identity after resetting. Reference: fullDeviceLoadedRef, lastDeviceIdRef
(create it), loadFullDevice, device.esiDeviceRef, and the existing useEffect
dependencies.

🧹 Nitpick comments (2)

src/renderer/components/_features/[workspace]/editor/device/ethercat/components/esi-repository.tsx (1)

93-127: Set explicit button types in the error panel controls.

Both buttons default to type='submit' in form contexts; this can trigger unintended submits.

🔧 Proposed fix

-            <button onClick={() => setErrorsExpanded((prev) => !prev)} className='flex items-center gap-2'>
+            <button type='button' onClick={() => setErrorsExpanded((prev) => !prev)} className='flex items-center gap-2'>
...
-            <button
+            <button
+              type='button'
               onClick={() => setUploadErrors([])}
               className='text-xs text-red-500 hover:text-red-700 dark:text-red-400 dark:hover:text-red-300'
             >

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/esi-repository.tsx
around lines 93 - 127, The two buttons in the error panel (the toggle that calls
setErrorsExpanded and the "Dismiss" button that calls setUploadErrors([])) lack
explicit type attributes and will act as type='submit' inside forms; update both
button elements to include type="button" to prevent accidental form submissions,
leaving their onClick handlers (setErrorsExpanded and setUploadErrors) unchanged
and ensuring the existing className and SVG content remain intact.

src/renderer/components/_features/[workspace]/editor/device/ethercat/components/configured-device-row.tsx (1)

25-57: Code duplication with device-detail-panel.tsx.

The types (EnrichDeviceData), constants (inputClassName, disabledInputClassName), and helper function (parseNumericInput) are duplicated from device-detail-panel.tsx. Consider extracting these into a shared module (e.g., @components/.../ethercat/utils/ or a shared constants.ts) to reduce maintenance burden.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/configured-device-row.tsx
around lines 25 - 57, The EnrichDeviceData type and duplicated UI
constants/functions (EnrichDeviceData, inputClassName, disabledInputClassName,
parseNumericInput) are repeated from device-detail-panel.tsx; extract these into
a shared module (e.g., a new ethercat utils/constants file under components or a
shared utils directory) and import them into configured-device-row.tsx and
device-detail-panel.tsx, updating both files to reference the centralized
EnrichDeviceData type and the shared inputClassName, disabledInputClassName, and
parseNumericInput exports to remove duplication and keep a single source of
truth.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/esi-repository.tsx:
- Around line 140-145: The table is receiving only isSaving and can render
prematurely; update the ESIRepositoryTable props to use the combined loading
state from the parent (e.g., replace the single isSaving prop with the combined
isLoading || isSaving or pass a new combinedLoading variable) so
ESIRepositoryTable receives the correct loading flag; locate the
ESIRepositoryTable usage and change the prop passed (keep existing handlers like
onRemoveItem/onClearAll intact).

---

Duplicate comments:
In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/configured-device-row.tsx:
- Around line 684-688: The conditional that shows the "No CoE Object Dictionary"
fallback incorrectly treats an empty coeObjects array as present; update the JSX
condition in the component (configured-device-row / the block that currently
checks !isLoadingChannels && !channelLoadError && !device.sdoConfigurations &&
!coeObjects) to instead check !coeObjects || coeObjects.length === 0 so an empty
array triggers the fallback; mirror the same check used by the "auto-config"
section which expects coeObjects.length > 0.
- Around line 107-175: fullDeviceLoadedRef currently never resets for a new
device and in-flight loads can race with updated dependencies; change
fullDeviceLoadedRef to store a device-identity key (e.g., combine
device.esiDeviceRef.repositoryItemId and device.esiDeviceRef.deviceIndex or a
device.id) and reset it when the identity changes, include that identity in the
effect deps, and add an in-flight cancellation token (AbortController or a local
"cancelled" flag checked after awaiting window.bridge.esiLoadDeviceFull in the
loadFullDevice function) so that if externalAddresses or the device identity
changes mid-load you bail out and don't apply stale setState or call
onEnrichDevice/onUpdateChannelMappings.
- Around line 201-240: The table row's click handler (onSelect) isn't
keyboard-accessible and the expand button (onToggleExpand / ArrowIcon) lacks
ARIA; make the row focusable and handle keyboard activation by adding tabIndex=0
to the <tr> and an onKeyDown that calls onSelect when Enter or Space is pressed
(mirror the existing onClick behavior), and add accessible attributes to the
expand button: include aria-expanded={isExpanded} and a descriptive aria-label
(e.g., `Expand ${device.name}`) while keeping e.stopPropagation() so the button
doesn't trigger row selection.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/device-detail-panel.tsx:
- Around line 623-655: The UI currently shows nothing when ESI returns an empty
array because the first branch checks coeObjects.length > 0 and the "no CoE"
message checks !coeObjects (which is false for []). Update the component logic:
either add an explicit branch that checks Array.isArray(coeObjects) &&
coeObjects.length === 0 and render the "No CoE Object Dictionary available..."
message, or change the existing final condition that uses !coeObjects to instead
use (!coeObjects || coeObjects.length === 0) so the empty-array case is handled;
reference the coeObjects variable and the surrounding conditions
isLoadingChannels, channelLoadError, and device.sdoConfigurations when making
this change.
- Around line 115-178: The guard fullDeviceLoadedRef prevents reload when the
device prop changes; compare the current device identity (e.g.,
device.esiDeviceRef.repositoryItemId + ':' + device.esiDeviceRef.deviceIndex) to
a stored lastDeviceIdRef inside the same useEffect and reset
fullDeviceLoadedRef.current = false when they differ before the early return, so
loadFullDevice runs for new devices; update lastDeviceIdRef with the new
identity after resetting. Reference: fullDeviceLoadedRef, lastDeviceIdRef
(create it), loadFullDevice, device.esiDeviceRef, and the existing useEffect
dependencies.

---

Nitpick comments:
In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/configured-device-row.tsx:
- Around line 25-57: The EnrichDeviceData type and duplicated UI
constants/functions (EnrichDeviceData, inputClassName, disabledInputClassName,
parseNumericInput) are repeated from device-detail-panel.tsx; extract these into
a shared module (e.g., a new ethercat utils/constants file under components or a
shared utils directory) and import them into configured-device-row.tsx and
device-detail-panel.tsx, updating both files to reference the centralized
EnrichDeviceData type and the shared inputClassName, disabledInputClassName, and
parseNumericInput exports to remove duplication and keep a single source of
truth.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/esi-repository.tsx:
- Around line 93-127: The two buttons in the error panel (the toggle that calls
setErrorsExpanded and the "Dismiss" button that calls setUploadErrors([])) lack
explicit type attributes and will act as type='submit' inside forms; update both
button elements to include type="button" to prevent accidental form submissions,
leaving their onClick handlers (setErrorsExpanded and setUploadErrors) unchanged
and ensuring the existing className and SVG content remain intact.

🪄 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: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: b2239528-5d76-4050-89a8-500895f686ee

📥 Commits

Reviewing files that changed from the base of the PR and between f6645fc and 52b8231.

📒 Files selected for processing (5)

• src/main/modules/ipc/main.ts
• src/renderer/components/_features/[workspace]/editor/device/ethercat/components/configured-device-row.tsx
• src/renderer/components/_features/[workspace]/editor/device/ethercat/components/device-detail-panel.tsx
• src/renderer/components/_features/[workspace]/editor/device/ethercat/components/esi-repository.tsx
• src/renderer/components/_features/[workspace]/editor/device/ethercat/components/sdo-parameters-table.tsx

✅ Files skipped from review due to trivial changes (1)

• src/main/modules/ipc/main.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/renderer/components/_features/[workspace]/editor/device/ethercat/components/sdo-parameters-table.tsx

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Use the combined loading state for the table as well.

ESIRepositoryTable currently ignores parent isLoading (Line 144), so it can render non-loading/empty UI while data is still loading.

🔧 Proposed fix

         <ESIRepositoryTable
           repository={repository}
           onRemoveItem={handleRemoveItem}
           onClearAll={handleClearAll}
-          isLoading={isSaving}
+          isLoading={isProcessing}
         />

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	<ESIRepositoryTable
	repository={repository}
	onRemoveItem={handleRemoveItem}
	onClearAll={handleClearAll}
	isLoading={isSaving}
	/>
	<ESIRepositoryTable
	repository={repository}
	onRemoveItem={handleRemoveItem}
	onClearAll={handleClearAll}
	isLoading={isProcessing}
	/>

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/esi-repository.tsx
around lines 140 - 145, The table is receiving only isSaving and can render
prematurely; update the ESIRepositoryTable props to use the combined loading
state from the parent (e.g., replace the single isSaving prop with the combined
isLoading || isSaving or pass a new combinedLoading variable) so
ESIRepositoryTable receives the correct loading flag; locate the
ESIRepositoryTable usage and change the prop passed (keep existing handlers like
onRemoveItem/onClearAll intact).

[coderabbitai]

♻️ Duplicate comments (1)

src/renderer/components/_features/[workspace]/editor/device/ethercat/components/configured-device-row.tsx (1)

201-220: ⚠️ Potential issue | 🟠 Major

Add keyboard accessibility for row selection and proper ARIA attributes for expand button.

The onSelect handler on <tr> is not keyboard-accessible since table rows aren't focusable by default. The expand button lacks aria-expanded and an accessible label. This blocks keyboard-only users from selecting devices or understanding the expand state.

♿ Proposed accessibility fix

       <tr
-        onClick={onSelect}
+        tabIndex={0}
+        role="button"
+        onClick={onSelect}
+        onKeyDown={(e) => {
+          if (e.key === 'Enter' || e.key === ' ') {
+            e.preventDefault()
+            onSelect()
+          }
+        }}
         className={cn(
           'cursor-pointer border-b border-neutral-200 dark:border-neutral-800',
           isSelected && 'bg-brand/10 dark:bg-brand/20',
         )}
       >
         <td className='px-2 py-2'>
           <button
             onClick={(e) => {
               e.stopPropagation()
               onToggleExpand()
             }}
             className='flex items-center justify-center'
+            aria-expanded={isExpanded}
+            aria-label={isExpanded ? 'Collapse device details' : 'Expand device details'}
           >

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/configured-device-row.tsx
around lines 201 - 220, The table row with the onSelect handler is not
keyboard-focusable and the expand button lacks ARIA attributes; make the <tr>
(the row that calls onSelect) keyboard-accessible by adding tabIndex={0} and a
keyDown handler that triggers onSelect for Enter and Space, or alternatively
move the onSelect logic to a focusable element inside the row, and ensure the
expand control (the button that calls onToggleExpand and renders ArrowIcon)
includes aria-expanded={isExpanded} and a clear accessible name (aria-label or
aria-labelledby) and, if there is an expandable region, aria-controls pointing
to that region's id so assistive tech can detect the expanded state.

🧹 Nitpick comments (5)

src/renderer/components/_features/[workspace]/editor/device/ethercat/components/esi-repository.tsx (2)

4-5: Use configured path aliases instead of relative imports.

Please switch these imports to the configured alias scheme to keep import resolution consistent across the renderer code.

♻️ Proposed change

-import { ESIRepositoryTable } from './esi-repository-table'
-import { ESIUpload } from './esi-upload'
+import { ESIRepositoryTable } from '@components/_features/[workspace]/editor/device/ethercat/components/esi-repository-table'
+import { ESIUpload } from '@components/_features/[workspace]/editor/device/ethercat/components/esi-upload'

As per coding guidelines, "Use path aliases as defined in tsconfig.json: @root/* for ./src/*, @process:main/* for ./src/main/*, @process:renderer/* for ./src/renderer/*, @components/* for ./src/renderer/components/*, @utils/* for ./src/utils/*, @shared/* for ./src/shared/*, @hooks/* for ./src/renderer/hooks/*".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/esi-repository.tsx
around lines 4 - 5, Replace the relative imports for ESIRepositoryTable and
ESIUpload with the configured path-alias imports (use the `@components` alias
which maps to src/renderer/components) so import statements reference the
components via `@components/`... instead of './...'; update the import lines that
reference the symbols ESIRepositoryTable and ESIUpload to use their aliased
module paths (e.g.
`@components/_features/`[workspace]/editor/device/ethercat/components/esi-repository-table
and
`@components/_features/`[workspace]/editor/device/ethercat/components/esi-upload).

---

46-49: Expose delete/clear failures in UI, not only in console logs.

Line 46 and Line 66 only log failures, so users can’t tell when an action fails. Consider adding a small inline error banner/toast state for mutation failures.

Also applies to: 66-69

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/esi-repository.tsx
around lines 46 - 49, The console-only failure handling in the delete and clear
mutation blocks (the console.error('Failed to delete ESI item:', result.error)
and console.error('Error deleting ESI item:', err) lines, and the analogous
clear failure logs around lines 66–69) should surface errors to users: add a
local UI error state (e.g., deleteError/clearError or a generic mutationError)
or invoke an existing toast/banner API from these handlers when result.error or
catch triggers, set the error text from result.error or err, and update the
component render to display a small inline banner/toast that clears on retry or
after a timeout so users see failures instead of only console logs.

src/renderer/components/_features/[workspace]/editor/device/ethercat/components/configured-device-row.tsx (3)

633-638: Consider using a dedicated spinner component instead of ArrowIcon.

Using ArrowIcon with animate-spin as a loading indicator is semantically confusing—an arrow doesn't communicate "loading" to users (or assistive technologies). A dedicated spinner or loading component would provide clearer visual feedback.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/configured-device-row.tsx
around lines 633 - 638, Replace the use of ArrowIcon inside the
isLoadingChannels conditional with a proper loading/spinner component (e.g.,
Spinner or LoadingSpinner) to improve semantics and accessibility; update the
JSX in the block that renders when isLoadingChannels to render the spinner
component (or create one) instead of ArrowIcon, keep the existing wrapper
classes ('flex items-center gap-2 py-4 text-sm ...') for layout, and ensure the
spinner component exposes accessible attributes such as role="status" and an
offscreen label or aria-live text like "Loading CoE data..." so assistive tech
will announce the loading state.

---

53-57: Consider adding max validation to parseNumericInput.

The function validates against min but not max, while some inputs specify a max (e.g., max={65535} on EtherCAT Address). While the HTML input provides browser-level validation, programmatically enforcing max would ensure robustness against edge cases (e.g., direct value manipulation or copy-paste).

🔧 Proposed enhancement

-const parseNumericInput = (value: string, min = 0): number | undefined => {
+const parseNumericInput = (value: string, min = 0, max?: number): number | undefined => {
   const parsed = parseInt(value, 10)
-  if (isNaN(parsed) || parsed < min) return undefined
+  if (isNaN(parsed) || parsed < min || (max !== undefined && parsed > max)) return undefined
   return parsed
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/configured-device-row.tsx
around lines 53 - 57, The parseNumericInput function only enforces a min but
should also accept an optional max to guard against values above allowed ranges;
change its signature to parseNumericInput(value: string, min = 0, max?: number)
and, after parsing, return undefined if parsed is NaN, parsed < min, or (max !==
undefined && parsed > max). Update any callers that rely on a specific upper
bound (e.g., the EtherCAT Address input using max={65535}) to pass that max
value into parseNumericInput so the value is programmatically validated as well
as by the HTML input.

---

110-175: Race condition mitigation appears adequate, but verify the "load once" behavior is intentional.

The isLoadingChannels guard prevents concurrent in-flight loads, and fullDeviceLoadedRef.current ensures the load happens only once per component instance. However, if externalAddresses changes after the initial load completes (e.g., another device's mappings change), channel mappings won't be regenerated.

If this "load once, never refresh" behavior is intentional for performance, consider adding a brief comment clarifying that design choice. If users expect mappings to update when external addresses change, you'd need a different approach.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/configured-device-row.tsx
around lines 110 - 175, The effect currently prevents reloading after the first
successful fetch via fullDeviceLoadedRef and isLoadingChannels, so changes to
externalAddresses won't trigger regeneration of default mappings; either add a
brief comment above the useEffect explaining the intentional "load once"
behavior, or implement a minimal refresh: add a prevExternalAddressesRef and
include a check in the effect to compare prevExternalAddressesRef.current with
externalAddresses and, if different and device.channelMappings.length === 0,
call onUpdateChannelMappings(generateDefaultChannelMappings(...)) (then update
prevExternalAddressesRef.current), keeping existing guards
(fullDeviceLoadedRef/isLoadingChannels) intact.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/configured-device-row.tsx:
- Around line 201-220: The table row with the onSelect handler is not
keyboard-focusable and the expand button lacks ARIA attributes; make the <tr>
(the row that calls onSelect) keyboard-accessible by adding tabIndex={0} and a
keyDown handler that triggers onSelect for Enter and Space, or alternatively
move the onSelect logic to a focusable element inside the row, and ensure the
expand control (the button that calls onToggleExpand and renders ArrowIcon)
includes aria-expanded={isExpanded} and a clear accessible name (aria-label or
aria-labelledby) and, if there is an expandable region, aria-controls pointing
to that region's id so assistive tech can detect the expanded state.

---

Nitpick comments:
In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/configured-device-row.tsx:
- Around line 633-638: Replace the use of ArrowIcon inside the isLoadingChannels
conditional with a proper loading/spinner component (e.g., Spinner or
LoadingSpinner) to improve semantics and accessibility; update the JSX in the
block that renders when isLoadingChannels to render the spinner component (or
create one) instead of ArrowIcon, keep the existing wrapper classes ('flex
items-center gap-2 py-4 text-sm ...') for layout, and ensure the spinner
component exposes accessible attributes such as role="status" and an offscreen
label or aria-live text like "Loading CoE data..." so assistive tech will
announce the loading state.
- Around line 53-57: The parseNumericInput function only enforces a min but
should also accept an optional max to guard against values above allowed ranges;
change its signature to parseNumericInput(value: string, min = 0, max?: number)
and, after parsing, return undefined if parsed is NaN, parsed < min, or (max !==
undefined && parsed > max). Update any callers that rely on a specific upper
bound (e.g., the EtherCAT Address input using max={65535}) to pass that max
value into parseNumericInput so the value is programmatically validated as well
as by the HTML input.
- Around line 110-175: The effect currently prevents reloading after the first
successful fetch via fullDeviceLoadedRef and isLoadingChannels, so changes to
externalAddresses won't trigger regeneration of default mappings; either add a
brief comment above the useEffect explaining the intentional "load once"
behavior, or implement a minimal refresh: add a prevExternalAddressesRef and
include a check in the effect to compare prevExternalAddressesRef.current with
externalAddresses and, if different and device.channelMappings.length === 0,
call onUpdateChannelMappings(generateDefaultChannelMappings(...)) (then update
prevExternalAddressesRef.current), keeping existing guards
(fullDeviceLoadedRef/isLoadingChannels) intact.

In
`@src/renderer/components/_features/`[workspace]/editor/device/ethercat/components/esi-repository.tsx:
- Around line 4-5: Replace the relative imports for ESIRepositoryTable and
ESIUpload with the configured path-alias imports (use the `@components` alias
which maps to src/renderer/components) so import statements reference the
components via `@components/`... instead of './...'; update the import lines that
reference the symbols ESIRepositoryTable and ESIUpload to use their aliased
module paths (e.g.
`@components/_features/`[workspace]/editor/device/ethercat/components/esi-repository-table
and
`@components/_features/`[workspace]/editor/device/ethercat/components/esi-upload).
- Around line 46-49: The console-only failure handling in the delete and clear
mutation blocks (the console.error('Failed to delete ESI item:', result.error)
and console.error('Error deleting ESI item:', err) lines, and the analogous
clear failure logs around lines 66–69) should surface errors to users: add a
local UI error state (e.g., deleteError/clearError or a generic mutationError)
or invoke an existing toast/banner API from these handlers when result.error or
catch triggers, set the error text from result.error or err, and update the
component render to display a small inline banner/toast that clears on retry or
after a timeout so users see failures instead of only console logs.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 838bd78e-96de-4227-88ac-4cac0f2c5143

📥 Commits

Reviewing files that changed from the base of the PR and between 52b8231 and fc91224.

📒 Files selected for processing (3)

• src/main/modules/ipc/main.ts
• src/renderer/components/_features/[workspace]/editor/device/ethercat/components/configured-device-row.tsx
• src/renderer/components/_features/[workspace]/editor/device/ethercat/components/esi-repository.tsx

🚧 Files skipped from review as they are similar to previous changes (1)

• src/main/modules/ipc/main.ts

[JoaoGSP]

~450+ lines of near-identical code duplicated between configured-device-row.tsx and this file: same EnrichDeviceData type, inputClassName/disabledInputClassName constants, parseNumericInput utility, useEffect for loadFullDevice, handleAliasChange/updateConfig callbacks, and the entire configuration form (Startup Checks, Addressing, Timeouts, Watchdog, Distributed Clocks SYNC0/SYNC1, SDO Parameters, Channel Mappings). The only difference is layout: table rows vs. tabbed panel.

Suggestion: Extract a shared DeviceConfigurationForm component and a useDeviceConfiguration hook to eliminate ~400-500 lines of duplication and prevent the two views from drifting out of sync.

[JoaoGSP]

This EnrichDeviceData type is defined identically in 4 separate files: configured-device-row.tsx, device-detail-panel.tsx, configured-devices.tsx, and devices-tab.tsx.

Define it once in a shared location (e.g. src/utils/ethercat/enrich-device-data.ts alongside the function that returns this shape, or src/types/ethercat/esi-types.ts) and import it in all four components.

[JoaoGSP]

Two complete ESI XML parsers with duplicated logic: this file (renderer, DOMParser) and esi-parser-main.ts (main process, fast-xml-parser). Both share identical parseHexValue, PDO entry parsing, FMMU/SM parsing, and device type extraction code.

Since all ESI parsing now routes through IPC to the main process parser, the renderer-side parseESI() function may be dead code. Its remaining exports (pdoToChannels, generateDefaultChannelMappings, generateIecLocation) operate on already-parsed ESIDevice objects, not XML — they're channel utilities, not parsers.

Suggestions:

1. Verify if parseESI() is still called anywhere; remove it if dead code
2. Move shared helpers like parseHexValue to a common utility
3. Rename this file to esi-channel-utils.ts to reflect its actual remaining purpose

[JoaoGSP]

delete enriched.sdoConfigurations mutates the return value of enrichDeviceData(). While it returns a fresh object today (so no shared-reference issues), this breaks if the function is ever memoized or cached, and is a code smell.

Prefer destructuring:

const { sdoConfigurations, ...rest } = enrichDeviceData(result.device)
onEnrichDevice(device.sdoConfigurations !== undefined ? rest : { ...rest, sdoConfigurations })

Same pattern appears in device-detail-panel.tsx.

[JoaoGSP]

This +74 line "Architecture Survey — Autonomy Logic" section is unrelated to the EtherCAT feature. It's also written in Portuguese while the rest of the codebase uses English. Consider removing it from this PR and adding it in a separate documentation commit.

[JoaoGSP]

This useEffect dependency array includes unstable references: externalAddresses (a new Set recomputed on every relevant re-render), callback props, and device.channelMappings.length. The fullDeviceLoadedRef guard prevents the async function from re-running, but the effect still re-enters on every dep change and hits the early return.

The fullDeviceLoadedRef pattern is fighting against React's dependency model — if any parent ever changes callback identity, the ref guard silently prevents re-loading even when it should.

Consider extracting this into a useDeviceConfiguration hook with dependencies limited to projectPath and device.esiDeviceRef, managing all loading/enrichment state internally. Same pattern exists in device-detail-panel.tsx.

[JoaoGSP]

src/main/modules/ipc/main.ts — 13 ESI IPC handlers follow an identical try/catch pattern wrapping a service call (~200 lines total). Consider a generic wrapper:

private wrapServiceCall<T>(fn: () => Promise<T>): Promise<T | { success: false; error: string }> {
  return fn().catch((error) => ({ success: false, error: String(error) }))
}

This would reduce the 13 handlers to ~40 lines.

Also, makeRuntimeApiPostRequest (this diff) is structurally very similar to the existing makeRuntimeApiRequest (GET). Both create an HTTPS request, handle timeout, parse response, and retry on token expiry. They could be a single method parameterized by method and optional body.