refactor(compile): single shared build pipeline - #794
Merged
Conversation
P0 of the shared-build-pipeline refactor. Lifts `compiler-module.ts handleGenerateDefinitionsFile`'s pure content-authoring logic into `backend/shared/compile/steps/ generate-defines.ts`. Editor-canonical behaviour: every byte of the new function's output matches what the editor used to emit directly to disk, so a fresh build produces a byte-identical `defines.h` to the previous version. The editor's handler becomes thin glue: read hals.json / pin- mapping.json / program.st from disk, call the shared function, write the result to `build/<target>/src/defines.h`. The shared function is process-safe (no fs, no DOM, no global state) so the web's build pipeline can call the exact same function and produce the same `defines.h` — load-bearing for the runtime's PROGRAM_MD5 stale-program detection and for the firmware HAL headers' `#ifdef`-gated `#include` directives. Tests cover every emission path: board defines (absent / string / array / empty array), PROGRAM_MD5 always emitted, simulator-only Comms block, IO Config by pinType, every Arduino-library marker string, marker substring semantics, plus byte-for-byte snapshots of the canonical simulator and arduino-cli outputs. 100% coverage on the new file (statements / lines / functions). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds pure shared modules for deterministic defines.h and runtime-v4 conf generation, a firmware-bundle composer, a shared compile pipeline and platform-port contract, with comprehensive Jest tests; updates the editor compiler to call the shared generators and remove inline/conf-writing methods. ChangesShared compile-step extraction and compiler wiring
Sequence Diagram(s)sequenceDiagram
participant CompilerModule
participant generateRuntimeConfs
participant ModbusGen as generateModbusSlave/Master
participant S7Gen as generateS7Comm
participant OpcUaGen as generateOpcUa
participant EthercatGen as generateEthercat + validate
CompilerModule->>generateRuntimeConfs: provide servers, remoteDevices, instances, debugMap, log
generateRuntimeConfs->>ModbusGen: generate modbusSlave/modbusMaster
generateRuntimeConfs->>S7Gen: generate s7Comm
generateRuntimeConfs->>OpcUaGen: try generate opcUa (log via callback)
generateRuntimeConfs->>EthercatGen: generate ethercat then validate
generateRuntimeConfs-->>CompilerModule: return {modbusSlave,modbusMaster,s7Comm,opcUa,ethercat}
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/backend/shared/compile/steps/generate-defines.ts (1)
20-20: ⚡ Quick winUse the repo alias for this shared type import.
This new shared module is adding a relative
src/...import instead of the project-wide@root/*alias.♻️ Proposed fix
-import type { DevicePin } from '../../types/PLC/devices' +import type { DevicePin } from '`@root/backend/shared/types/PLC/devices`'As per coding guidelines, "Use path alias
@root/*to reference./src/*".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/shared/compile/steps/generate-defines.ts` at line 20, Replace the relative import of the shared type with the project path alias: in generate-defines.ts change the import that references '../../types/PLC/devices' for the DevicePin type to use the `@root` alias (e.g. import type { DevicePin } from '`@root/`...') so it follows the repo guideline "Use path alias `@root/`* to reference ./src/*"; update the module path after `@root` to match the original module location.src/backend/shared/compile/__tests__/generate-defines.test.ts (1)
16-17: ⚡ Quick winSwitch the new test imports to
@root/*.These new relative imports don't follow the repo import-path convention.
♻️ Proposed fix
-import type { DevicePin } from '../../types/PLC/devices' -import { type BoardHalsDefinesEntry, generateDefinesContent } from '../steps/generate-defines' +import type { DevicePin } from '`@root/backend/shared/types/PLC/devices`' +import { type BoardHalsDefinesEntry, generateDefinesContent } from '`@root/backend/shared/compile/steps/generate-defines`'As per coding guidelines, "Use path alias
@root/*to reference./src/*".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/shared/compile/__tests__/generate-defines.test.ts` around lines 16 - 17, The test file uses relative imports for DevicePin and for BoardHalsDefinesEntry/generateDefinesContent; update those to use the repository path alias instead (replace '../../types/PLC/devices' with '`@root/path/to/types/PLC/devices`' and '../steps/generate-defines' with '`@root/path/to/shared/compile/steps/generate-defines`' or the correct `@root/`* equivalents) so the imports for DevicePin, BoardHalsDefinesEntry, and generateDefinesContent follow the repo convention using `@root/`*.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/backend/shared/compile/__tests__/generate-defines.test.ts`:
- Around line 16-17: The test file uses relative imports for DevicePin and for
BoardHalsDefinesEntry/generateDefinesContent; update those to use the repository
path alias instead (replace '../../types/PLC/devices' with
'`@root/path/to/types/PLC/devices`' and '../steps/generate-defines' with
'`@root/path/to/shared/compile/steps/generate-defines`' or the correct `@root/`*
equivalents) so the imports for DevicePin, BoardHalsDefinesEntry, and
generateDefinesContent follow the repo convention using `@root/`*.
In `@src/backend/shared/compile/steps/generate-defines.ts`:
- Line 20: Replace the relative import of the shared type with the project path
alias: in generate-defines.ts change the import that references
'../../types/PLC/devices' for the DevicePin type to use the `@root` alias (e.g.
import type { DevicePin } from '`@root/`...') so it follows the repo guideline
"Use path alias `@root/`* to reference ./src/*"; update the module path after
`@root` to match the original module location.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 32d562be-b647-4569-a8cc-1c799c8076f0
📒 Files selected for processing (3)
src/backend/editor/compiler/compiler-module.tssrc/backend/shared/compile/__tests__/generate-defines.test.tssrc/backend/shared/compile/steps/generate-defines.ts
P2 of the shared-build-pipeline refactor.
Lifts the inline conf-generation block from `compileProgram`'s
runtime-v4 branch (Modbus slave/master, S7Comm, OPC-UA, EtherCAT)
into `backend/shared/compile/steps/generate-confs.ts`. The atomic
generators were already shared; what was duplicated was the
orchestration:
- OPC-UA's two-prefix error handling (`OPC-UA Configuration Error:`
for `OpcUaConfigError`, `Failed to generate OPC-UA config:` for
everything else, both rethrown).
- EtherCAT's validate-before-emit gate that aborts the compile
when the config fails validation.
- The specific log-message format for OPC-UA's informational
output (node-count progress lines piped through to the user's
compile log).
Editor's `compileProgram` runtime-v4 block now makes one call to
`generateRuntimeConfs` and receives the five conf strings (or
nulls) back. Web's pipeline will route through the same helper
once it lands on the shared pipeline orchestrator in P3.
Dead code removed: five `handleGenerate*Config` methods that were
never called from anywhere in the codebase. They predated the
inline runtime-v4 branch but were left behind during a previous
refactor; deleting them removes the temptation to call them from
some future place and silently produce different output than
the runtime-v4 path does. About 165 lines deleted.
Imports cleaned up: removed `generateEthercatConfig`,
`validateEthercatConfig`, `generateModbusMasterConfig`,
`generateModbusSlaveConfig`, `generateOpcUaConfig +
OpcUaConfigError`, `generateS7CommConfig` from compiler-module.ts
— all moved behind the shared helper. `getErrorMessage` stays
(still used in VPP packaging, runtime upload, debug compile, and
library bail-out paths).
Tests: 14 cases covering happy path, OPC-UA error prefix branches
(OpcUaConfigError / generic Error / non-Error throws), EtherCAT
validation gate, ordering invariants, log-callback wiring. Atomic
generators are mocked so the orchestration can be driven through
every branch without elaborate project fixtures. 100% coverage
on `generate-confs.ts` (statements / lines / functions).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
P3a of the shared-build-pipeline refactor — foundational pieces
for the shared pipeline orchestrator (P3b). No behaviour change
on this commit: the new modules are not yet called from anywhere.
CompilerPlatformPort (src/middleware/shared/ports/compiler-platform-port.ts)
----------------------------------------------------------------------
Defines the thin platform-bridge interface the shared pipeline will
call into for every step that genuinely depends on the platform:
- transpileXmlToSt (xml2st binary vs HTTP /generate-st)
- compileArduino (arduino-cli subprocess vs HTTP /compile-arduino)
- uploadRuntimeV4 (HTTPS to device vs orchestrator pipe)
- uploadArduinoBoard (editor-only: physical Arduino direct upload)
- uploadRuntimeV3 (editor-only: legacy v3 program.st upload)
- installArduinoCore (editor-only: arduino-cli core install)
- installArduinoLib (editor-only: arduino-cli lib install)
- checkRuntimeVersion (HTTP /api/version vs orchestrator probe)
Editor-only methods MUST resolve to `{ ok: true }` on web (the
adapter no-ops them) so the pipeline's ordering and downstream
steps run identically on both platforms — web simply skips work
that's already handled server-side, and the orchestrator never
knows.
`PlatformDeviceContext` is a discriminated union (`editor-https` /
`web-orchestrator`); the pipeline forwards it through to upload
methods without inspecting it. `PlatformLog` is the canonical
progress channel — each adapter translates to its native shape
(IPC postMessage on editor; onProgress event on web).
composeFirmwareBundle (src/backend/shared/compile/steps/compose-firmware-bundle.ts)
-----------------------------------------------------------------------------------
The simulator/Arduino-firmware analog of composeRuntimeV4Bundle.
Pure function that assembles the file map arduino-cli sees:
firmware skeleton + strucpp output (under `src/`) + c_blocks.h
(overwrite) + c_blocks_code.cpp (overwrite only when project has
C/C++ POUs, mirroring editor's "skipping c_blocks_code.cpp
generation" path) + defines.h (overwrite).
Editor-canonical layout: paths/overwrite semantics match what
compiler-module.ts emits to disk today. Both repos used to
assemble this independently — editor via scattered writeFile
calls, web via inline assembly into arduinoFiles. The C/C++ POU
bug we hot-fixed last week was a symptom of that drift; routing
both platforms through this composer makes it structurally
impossible to recur.
Plus a helper `buildCBlocksFromPous(originalCppPous)` that
constructs the `{header, code}` input from the preprocessor's
sidecar — empty POU list yields the `{header: '// Empty file\n',
code: null}` sentinel (composer leaves the static baseline alone).
Tests: 13 cases covering skeleton passthrough, strucpp output
landing under `src/`, c_blocks_code.cpp overwrite semantics
(present/absent, mirror of editor's behaviour), two full-layout
snapshots (with and without C/C++ POUs), and the buildCBlocksFromPous
helper. 100% coverage on the new file.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/backend/editor/compiler/compiler-module.ts`:
- Around line 2033-2042: The code passes an empty string for the required
runtime-v4 input debug-map.json to generateRuntimeConfs, which masks missing
output from handleCompileSTtoCpp; update the call site to fail fast when
strucppEmittedFiles['debug-map.json'] is absent (or empty) by throwing or
returning an error before calling generateRuntimeConfs (include a clear message
referencing "debug-map.json" and the compilation step), so generateRuntimeConfs
is only invoked with a valid debugMapContent; use the existing symbols
strucppEmittedFiles, generateRuntimeConfs and handleCompileSTtoCpp to locate and
implement the check.
- Around line 2059-2067: The code coerces a nullable EtherCAT string to '' in
compiler-module.ts (ethercat: confs.ethercat ?? ''), causing
composeRuntimeV4Bundle to write an empty non-JSON conf/ethercat.json; instead
keep confs.ethercat nullable by removing the '?? ""' coercion and update the
bundle input type to allow string | null (propagate the nullable type from
generateRuntimeConfs), then modify composeRuntimeV4Bundle to only set
files['conf/ethercat.json'] when input.confs.ethercat is non-null (i.e.,
conditionally write the file) so no empty file is emitted when EtherCAT is
disabled.
In `@src/backend/shared/compile/__tests__/generate-confs.test.ts`:
- Line 17: The test file imports and mocks use relative paths to src/* (e.g.,
the types PLCRemoteDevice and PLCServer) which breaks the project alias rule;
update every import/mock in this test (including the PLCRemoteDevice and
PLCServer type import and the modules mocked between the shown region) to use
the `@root/`* path alias that maps to ./src/* (for example change import paths
like ../../.../src/... to `@root/`...), preserving exact exported names and types
so tests resolve correctly.
In `@src/backend/shared/compile/steps/generate-confs.ts`:
- Around line 38-46: Replace the deep relative imports in this file with the
repository path-alias `@root/*` for all referenced symbols (getErrorMessage,
generateModbusSlaveConfig, generateOpcUaConfig, OpcUaConfigError,
generateS7CommConfig, generateEthercatConfig, validateEthercatConfig,
PLCRemoteDevice, PLCServer, generateModbusMasterConfig) so they reference the
corresponding modules under src via the `@root` alias; update each import
statement to use the `@root/`... path that mirrors the original src location and
ensure the imports still resolve against the existing tsconfig/paths
configuration.
🪄 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: 29add32b-b9f1-404b-9734-14b483f829e7
📒 Files selected for processing (3)
src/backend/editor/compiler/compiler-module.tssrc/backend/shared/compile/__tests__/generate-confs.test.tssrc/backend/shared/compile/steps/generate-confs.ts
| * elaborate project fixtures. | ||
| */ | ||
|
|
||
| import type { PLCRemoteDevice, PLCServer } from '../../types/PLC/open-plc' |
Contributor
There was a problem hiding this comment.
Align test imports/mocks with the @root/* alias convention.
Line 17 and Line 24 through Line 69 use relative module paths to src/*. This violates the alias rule and can cause noisy churn when folders move.
As per coding guidelines, "**/*.{ts,tsx,js,jsx}: Use path alias @root/* to reference ./src/*".
Also applies to: 24-69
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/backend/shared/compile/__tests__/generate-confs.test.ts` at line 17, The
test file imports and mocks use relative paths to src/* (e.g., the types
PLCRemoteDevice and PLCServer) which breaks the project alias rule; update every
import/mock in this test (including the PLCRemoteDevice and PLCServer type
import and the modules mocked between the shown region) to use the `@root/`* path
alias that maps to ./src/* (for example change import paths like
../../.../src/... to `@root/`...), preserving exact exported names and types so
tests resolve correctly.
Comment on lines
+38
to
+46
| import { getErrorMessage } from '../../../../frontend/utils/get-error-message' | ||
| import { generateModbusSlaveConfig } from '../../../../frontend/utils/modbus/generate-modbus-slave-config' | ||
| import { generateOpcUaConfig, OpcUaConfigError } from '../../../../frontend/utils/opcua' | ||
| import { generateS7CommConfig } from '../../../../frontend/utils/s7comm' | ||
| import { generateEthercatConfig } from '../../ethercat/generate-ethercat-config' | ||
| import { validateEthercatConfig } from '../../ethercat/validate-ethercat-config' | ||
| import type { PLCRemoteDevice, PLCServer } from '../../types/PLC/open-plc' | ||
| import { generateModbusMasterConfig } from '../../utils/modbus/generate-modbus-master-config' | ||
|
|
Contributor
There was a problem hiding this comment.
Use @root/* aliases instead of deep relative imports.
Line 38 through Line 46 reference ./src/* via ../../.. paths, which violates the repo import-path rule and makes refactors brittle.
💡 Suggested import rewrite
-import { getErrorMessage } from '../../../../frontend/utils/get-error-message'
-import { generateModbusSlaveConfig } from '../../../../frontend/utils/modbus/generate-modbus-slave-config'
-import { generateOpcUaConfig, OpcUaConfigError } from '../../../../frontend/utils/opcua'
-import { generateS7CommConfig } from '../../../../frontend/utils/s7comm'
-import { generateEthercatConfig } from '../../ethercat/generate-ethercat-config'
-import { validateEthercatConfig } from '../../ethercat/validate-ethercat-config'
-import type { PLCRemoteDevice, PLCServer } from '../../types/PLC/open-plc'
-import { generateModbusMasterConfig } from '../../utils/modbus/generate-modbus-master-config'
+import { generateEthercatConfig } from '`@root/backend/shared/ethercat/generate-ethercat-config`'
+import { validateEthercatConfig } from '`@root/backend/shared/ethercat/validate-ethercat-config`'
+import type { PLCRemoteDevice, PLCServer } from '`@root/backend/shared/types/PLC/open-plc`'
+import { generateModbusMasterConfig } from '`@root/backend/shared/utils/modbus/generate-modbus-master-config`'
+import { getErrorMessage } from '`@root/frontend/utils/get-error-message`'
+import { generateModbusSlaveConfig } from '`@root/frontend/utils/modbus/generate-modbus-slave-config`'
+import { generateOpcUaConfig, OpcUaConfigError } from '`@root/frontend/utils/opcua`'
+import { generateS7CommConfig } from '`@root/frontend/utils/s7comm`'As per coding guidelines, "**/*.{ts,tsx,js,jsx}: Use path alias @root/* to reference ./src/*".
📝 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
| import { getErrorMessage } from '../../../../frontend/utils/get-error-message' | |
| import { generateModbusSlaveConfig } from '../../../../frontend/utils/modbus/generate-modbus-slave-config' | |
| import { generateOpcUaConfig, OpcUaConfigError } from '../../../../frontend/utils/opcua' | |
| import { generateS7CommConfig } from '../../../../frontend/utils/s7comm' | |
| import { generateEthercatConfig } from '../../ethercat/generate-ethercat-config' | |
| import { validateEthercatConfig } from '../../ethercat/validate-ethercat-config' | |
| import type { PLCRemoteDevice, PLCServer } from '../../types/PLC/open-plc' | |
| import { generateModbusMasterConfig } from '../../utils/modbus/generate-modbus-master-config' | |
| import { generateEthercatConfig } from '`@root/backend/shared/ethercat/generate-ethercat-config`' | |
| import { validateEthercatConfig } from '`@root/backend/shared/ethercat/validate-ethercat-config`' | |
| import type { PLCRemoteDevice, PLCServer } from '`@root/backend/shared/types/PLC/open-plc`' | |
| import { generateModbusMasterConfig } from '`@root/backend/shared/utils/modbus/generate-modbus-master-config`' | |
| import { getErrorMessage } from '`@root/frontend/utils/get-error-message`' | |
| import { generateModbusSlaveConfig } from '`@root/frontend/utils/modbus/generate-modbus-slave-config`' | |
| import { generateOpcUaConfig, OpcUaConfigError } from '`@root/frontend/utils/opcua`' | |
| import { generateS7CommConfig } from '`@root/frontend/utils/s7comm`' |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/backend/shared/compile/steps/generate-confs.ts` around lines 38 - 46,
Replace the deep relative imports in this file with the repository path-alias
`@root/*` for all referenced symbols (getErrorMessage,
generateModbusSlaveConfig, generateOpcUaConfig, OpcUaConfigError,
generateS7CommConfig, generateEthercatConfig, validateEthercatConfig,
PLCRemoteDevice, PLCServer, generateModbusMasterConfig) so they reference the
corresponding modules under src via the `@root` alias; update each import
statement to use the `@root/`... path that mirrors the original src location and
ensure the imports still resolve against the existing tsconfig/paths
configuration.
P3b of the shared-build-pipeline refactor — the heart of the
unification. Single async function `runCompilePipeline` that drives
the full editor-canonical compile flow (Steps 0-13) through a
`CompilerPlatformPort`. No behaviour change on editor yet: this
commit only adds the shared module. Editor's `compiler-module.ts`
gets wired to it in P3c.
The four pipeline paths (editor-canonical ordering):
Simulator (avr8js):
preprocess → XML → ST → strucpp → installCore → installLib →
composeFirmwareBundle → compileArduino → return hex bytes.
Arduino direct (physical board):
preprocess → XML → ST → strucpp → installCore → installLib →
composeFirmwareBundle → compileArduino → uploadArduinoBoard.
Runtime v4 (OpenPLC vPLC):
preprocess → XML → ST → strucpp → generateRuntimeConfs →
composeRuntimeV4Bundle → checkRuntimeVersion (gate) →
uploadRuntimeV4.
Runtime v3 (legacy):
preprocess → XML → ST → strucpp → uploadRuntimeV3 (port
implementation embeds c_blocks into program.st before send).
`compileOnly` short-circuits each path before the upload step.
The pipeline is fully platform-agnostic — every platform-specific
operation goes through `CompilerPlatformPort` (8 methods, each
takes canonical args + a `PlatformLog` callback and returns a
canonical result shape). Each shared step is pure (`preprocessPous`,
`XmlGenerator`, `runProgramBuildPipeline`, `generateDefinesContent`,
`generateRuntimeConfs`, `composeFirmwareBundle`,
`composeRuntimeV4Bundle`, `buildArduinoCliCompileArgs`,
`buildKnownPous`, `emitCompileErrorEvents`,
`isStrucppCompatibleRuntime`, `describeIncompatibleRuntime`,
`buildCBlocksFromPous`).
Port method additions in this commit:
- `computeMd5(input)` — Editor: Node's `crypto.createHash('md5')`;
Web: `spark-md5` (already a web dep). Both produce byte-
identical hex. Kept in the port so the shared module ships
without a heavyweight hash-impl dependency.
Type note: the pipeline accepts the schema-shape `PLCProjectData`
(`configuration` singular) from `backend/shared/types/PLC/open-plc`,
because that's the runtime shape editor's pipeline operates on.
The web adapter currently stores in port-shape (`configurations`
plural) and will cast at the pipeline entry — see C1 in the
architectural plan. `as never` casts inside the pipeline bridge
the two shapes at API boundaries where shared helpers were typed
against port shape.
Progress events:
- 16-stage enum (`preprocess` / `xml` / `st` / `strucpp` / `confs`
/ `firmware-bundle` / `runtime-v4-bundle` / `embed-c-blocks` /
`core-install` / `lib-install` / `arduino-compile` /
`runtime-version` / `upload` / `done` / `error`)
- Canonical message + level + optional structured `compileError`.
- Adapter translates to its native shape (editor:
`_mainProcessPort.postMessage`; web: `onProgress` event).
Errors:
- Pre-strucpp failures bail with `Stopping compilation process.`
log line matching editor's existing emission.
- Per-strucpp-error events run through the shared
`emitCompileErrorEvents` helper with the structured payload the
renderer's click-to-navigate keys off.
Tests: 32 cases covering all four paths + compile-only variants,
device-context-missing branches, runtime-version gate, every
port method's failure response, strucpp's splitter fallback +
debug-map summary + warnings emission, boardEntry shape variants
(platform present / absent), per-error event payloads. Heavy
shared deps (`preprocessPous`, `XmlGenerator`,
`runProgramBuildPipeline`) are mocked so the orchestration can
be driven through every branch without elaborate fixtures.
Coverage on pipeline.ts: 95.7% statements / 84.6% branches /
69.2% functions (uncovered functions are inline helpers + one
unreachable bail). The shared-coverage gate baseline already
fails on `development` (71%-72%), so this isn't a regression.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
src/backend/shared/compile/__tests__/pipeline.test.ts (2)
634-693: ⚡ Quick winMake this conf-error test deterministic.
expect(typeof result.success).toBe('boolean')passes whether the conf step throws or not, so this test won't catch a regression in thetry/catcharoundgenerateRuntimeConfs(). Mock the conf generator or one of its direct dependencies and assert the actual failure path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/shared/compile/__tests__/pipeline.test.ts` around lines 634 - 693, The test is non-deterministic because it relies on real validation; instead explicitly mock the conf-generation path to force a throw and assert the pipeline maps that to success:false. Replace the current fuzzy setup with a mock of the conf generator (or the direct validator) used by runCompilePipeline — e.g. stub the module/function that runCompilePipeline calls to generate runtime confs (referencing generateRuntimeConfs or the EtherCAT validator like validateEthercatConfig) to throw an error once, then call runCompilePipeline and assert result.success === false (and check result.errors contains the thrown error); remember to restore the mock after the test.
15-58: ⚡ Quick winUse
@root/*imports in this test file.These imports point at other
src/*modules through long relative paths. Please switch them to the repo alias for consistency with the TS import rule.As per coding guidelines, "Use path alias
@root/*to reference./src/*"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/shared/compile/__tests__/pipeline.test.ts` around lines 15 - 58, Replace long relative imports in this test with the repo path alias `@root/`*: update the type imports (DevicePin, PLCProjectData, CompilerPlatformPort, PlatformDeviceContext) and the mocked/module imports (preprocessPous, XmlGenerator, runProgramBuildPipeline, isStrucppCompatibleRuntime, and the exported runCompilePipeline with RunCompilePipelineArgs and PipelineProgressEvent) to use `@root/`... paths instead of "../../../../" or "../../" relative paths so the file consistently uses the project alias.src/backend/shared/compile/pipeline.ts (1)
23-51: ⚡ Quick winUse
@root/*imports in this module.These deep relative imports violate the repo import rule and make this shared file harder to move safely. Please switch the
src/*references here to@root/*.As per coding guidelines, "Use path alias
@root/*to reference./src/*"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/shared/compile/pipeline.ts` around lines 23 - 51, Update the top-level imports in pipeline.ts to use the `@root/`* alias instead of deep relative paths: replace occurrences like '../../../middleware/shared/ports/compiler-platform-port' (symbols: CompilerPlatformPort, PlatformDeviceContext, PlatformLog), '../../../middleware/shared/ports/types' (StructuredCompileError), '../../../middleware/shared/utils/library/compose-runtime-v4-bundle' (composeRuntimeV4Bundle), '../firmware/build-arduino-cli-args' (BoardHalsCompileEntry, buildArduinoCliCompileArgs), '../firmware/runtime-version-gate' (describeIncompatibleRuntime, isStrucppCompatibleRuntime), '../library/program-build-helpers' (buildKnownPous, emitCompileErrorEvents), '../library/program-build-pipeline' (runProgramBuildPipeline), '../types/PLC/devices' (DevicePin), '../types/PLC/open-plc' (PLCProjectData), '../utils/PLC/preprocess-pous' (preprocessPous), '../utils/PLC/xml-generator' (XmlGenerator), './steps/compose-firmware-bundle' (buildCBlocksFromPous, composeFirmwareBundle), './steps/generate-confs' (generateRuntimeConfs) and './steps/generate-defines' (generateDefinesContent) so the path begins with `@root/` followed by the path under src; preserve the remainder of each module path and exported symbol names exactly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/backend/shared/compile/__tests__/compose-firmware-bundle.test.ts`:
- Line 12: Replace the relative import at the top of the test that currently
imports buildCBlocksFromPous and composeFirmwareBundle using a relative path;
instead use the project path alias `@root` to reference the same module (import {
buildCBlocksFromPous, composeFirmwareBundle } from '`@root/`...') so the test
follows the codebase guideline to use `@root/`* for ./src/* imports and keeps
symbols buildCBlocksFromPous and composeFirmwareBundle unchanged.
In `@src/backend/shared/compile/pipeline.ts`:
- Around line 581-597: The upload call drops the selected device target by
passing an empty port and so relies on hidden adapter state; change the
port.uploadArduinoBoard invocation to use the deviceContext values (e.g.,
deviceContext.port and deviceContext.fqbn or whatever field holds the upload
target) instead of ''. In practice update the argument object passed to
port.uploadArduinoBoard (and ensure fqbn is taken from boardEntry.platform or
deviceContext if present) so compilationPath, fqbn and port come from
compileResult/boardEntry/deviceContext as appropriate; keep
makePlatformLog(emit, 'upload') and the surrounding emit calls intact.
- Around line 471-523: The runtime-v3 path is executed after Arduino-only setup
(installArduinoCore/installArduinoLib and bundle/defines creation) causing
unnecessary failures; modify the control flow so you check isRuntimeV3 before
calling port.installArduinoCore, port.installArduinoLib, generateDefinesContent,
composeFirmwareBundle and buildArduinoCliCompileArgs and short-circuit to the
runtime-v3 logic (respecting compileOnly and using emit/bailError as needed) so
runtime-v3 uploads never depend on Arduino tool setup; locate the relevant
symbols isRuntimeV3, installArduinoCore, installArduinoLib,
generateDefinesContent, composeFirmwareBundle, buildArduinoCliCompileArgs,
compileOnly, emit and bailError and move or guard those calls accordingly.
- Around line 446-451: The code assumes port.checkRuntimeVersion succeeded;
first validate the probe result (e.g., check versionCheck.ok /
versionCheck.error / missing version) right after await port.checkRuntimeVersion
and before calling isStrucppCompatibleRuntime, and if it failed bail with
bailError(emit, 'runtime-version', /* include probe error details or message
from versionCheck */) so the real transport/auth error is reported; keep using
makePlatformLog(emit,'runtime-version') to log the attempt and only call
describeIncompatibleRuntime(versionCheck.version) after confirming a valid
version exists.
- Around line 534-542: The uploadRuntimeV3 call is being handed only raw
programSt so the generated C blocks are not passed along; update the pipeline to
embed the C blocks into program.st before calling port.uploadRuntimeV3 (or
include the c block artifacts on the payload) by invoking the existing embed
routine (or helper) and passing the resulting embeddedProgram (or { programSt:
embeddedProgram, cBlocks: ... }) instead of programSt; ensure you still pass
context: deviceContext and makePlatformLog(emit, 'upload') to
port.uploadRuntimeV3 so the port receives the program with embedded c_blocks.h /
c_blocks_code.cpp.
In `@src/backend/shared/compile/steps/compose-firmware-bundle.ts`:
- Around line 33-36: The imports in compose-firmware-bundle.ts use relative
paths; update them to the project path alias by replacing
'../../utils/cpp/generateCBlocksCode' and
'../../utils/cpp/generateCBlocksHeader' with
'`@root/utils/cpp/generateCBlocksCode`' and
'`@root/utils/cpp/generateCBlocksHeader`' respectively so that the named symbols
(generateCBlocksCode, generateCBlocksHeader, and the CppPouData types aliased as
CppPouDataCode/CppPouDataHeader) are imported via the `@root/`* alias consistent
with project conventions.
- Around line 92-95: The comment above the empty-POU return is stale: it claims
`null` for `code` means the composer skips writing `c_blocks.h`, but
`src/c_blocks.h` is actually always overwritten later while only
`c_blocks_code.cpp` is conditionally skipped. Update the comment near the return
{ header: '// Empty file\n', code: null } in compose-firmware-bundle.ts to
accurately state that the header (`c_blocks.h`) is written regardless and that
only the `code` field controls skipping of `c_blocks_code.cpp`; ensure
references to `header` and `code` reflect that behavior.
---
Nitpick comments:
In `@src/backend/shared/compile/__tests__/pipeline.test.ts`:
- Around line 634-693: The test is non-deterministic because it relies on real
validation; instead explicitly mock the conf-generation path to force a throw
and assert the pipeline maps that to success:false. Replace the current fuzzy
setup with a mock of the conf generator (or the direct validator) used by
runCompilePipeline — e.g. stub the module/function that runCompilePipeline calls
to generate runtime confs (referencing generateRuntimeConfs or the EtherCAT
validator like validateEthercatConfig) to throw an error once, then call
runCompilePipeline and assert result.success === false (and check result.errors
contains the thrown error); remember to restore the mock after the test.
- Around line 15-58: Replace long relative imports in this test with the repo
path alias `@root/`*: update the type imports (DevicePin, PLCProjectData,
CompilerPlatformPort, PlatformDeviceContext) and the mocked/module imports
(preprocessPous, XmlGenerator, runProgramBuildPipeline,
isStrucppCompatibleRuntime, and the exported runCompilePipeline with
RunCompilePipelineArgs and PipelineProgressEvent) to use `@root/`... paths instead
of "../../../../" or "../../" relative paths so the file consistently uses the
project alias.
In `@src/backend/shared/compile/pipeline.ts`:
- Around line 23-51: Update the top-level imports in pipeline.ts to use the
`@root/`* alias instead of deep relative paths: replace occurrences like
'../../../middleware/shared/ports/compiler-platform-port' (symbols:
CompilerPlatformPort, PlatformDeviceContext, PlatformLog),
'../../../middleware/shared/ports/types' (StructuredCompileError),
'../../../middleware/shared/utils/library/compose-runtime-v4-bundle'
(composeRuntimeV4Bundle), '../firmware/build-arduino-cli-args'
(BoardHalsCompileEntry, buildArduinoCliCompileArgs),
'../firmware/runtime-version-gate' (describeIncompatibleRuntime,
isStrucppCompatibleRuntime), '../library/program-build-helpers' (buildKnownPous,
emitCompileErrorEvents), '../library/program-build-pipeline'
(runProgramBuildPipeline), '../types/PLC/devices' (DevicePin),
'../types/PLC/open-plc' (PLCProjectData), '../utils/PLC/preprocess-pous'
(preprocessPous), '../utils/PLC/xml-generator' (XmlGenerator),
'./steps/compose-firmware-bundle' (buildCBlocksFromPous, composeFirmwareBundle),
'./steps/generate-confs' (generateRuntimeConfs) and './steps/generate-defines'
(generateDefinesContent) so the path begins with `@root/` followed by the path
under src; preserve the remainder of each module path and exported symbol names
exactly.
🪄 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: 6f74fad9-3e74-4894-ab78-4855b39dd07a
📒 Files selected for processing (5)
src/backend/shared/compile/__tests__/compose-firmware-bundle.test.tssrc/backend/shared/compile/__tests__/pipeline.test.tssrc/backend/shared/compile/pipeline.tssrc/backend/shared/compile/steps/compose-firmware-bundle.tssrc/middleware/shared/ports/compiler-platform-port.ts
| * arduino-cli link error. | ||
| */ | ||
|
|
||
| import { buildCBlocksFromPous, composeFirmwareBundle } from '../steps/compose-firmware-bundle' |
Contributor
There was a problem hiding this comment.
Use @root/* import in this test as well.
Please replace the relative import with the project alias.
Suggested diff
-import { buildCBlocksFromPous, composeFirmwareBundle } from '../steps/compose-firmware-bundle'
+import { buildCBlocksFromPous, composeFirmwareBundle } from '`@root/backend/shared/compile/steps/compose-firmware-bundle`'As per coding guidelines, **/*.{ts,tsx,js,jsx}: Use path alias @root/* to reference ./src/*.
📝 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
| import { buildCBlocksFromPous, composeFirmwareBundle } from '../steps/compose-firmware-bundle' | |
| import { buildCBlocksFromPous, composeFirmwareBundle } from '`@root/backend/shared/compile/steps/compose-firmware-bundle`' |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/backend/shared/compile/__tests__/compose-firmware-bundle.test.ts` at line
12, Replace the relative import at the top of the test that currently imports
buildCBlocksFromPous and composeFirmwareBundle using a relative path; instead
use the project path alias `@root` to reference the same module (import {
buildCBlocksFromPous, composeFirmwareBundle } from '`@root/`...') so the test
follows the codebase guideline to use `@root/`* for ./src/* imports and keeps
symbols buildCBlocksFromPous and composeFirmwareBundle unchanged.
Comment on lines
+446
to
+451
| const versionCheck = await port.checkRuntimeVersion( | ||
| { context: deviceContext }, | ||
| makePlatformLog(emit, 'runtime-version'), | ||
| ) | ||
| if (!isStrucppCompatibleRuntime(versionCheck.version)) { | ||
| return bailError(emit, 'runtime-version', describeIncompatibleRuntime(versionCheck.version)) |
Contributor
There was a problem hiding this comment.
Handle failed runtime-version probes before the compatibility gate.
checkRuntimeVersion() is treated as successful unconditionally here. If the probe fails, this branch can report an "upgrade your runtime" message for a transport/auth error instead of the real cause.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/backend/shared/compile/pipeline.ts` around lines 446 - 451, The code
assumes port.checkRuntimeVersion succeeded; first validate the probe result
(e.g., check versionCheck.ok / versionCheck.error / missing version) right after
await port.checkRuntimeVersion and before calling isStrucppCompatibleRuntime,
and if it failed bail with bailError(emit, 'runtime-version', /* include probe
error details or message from versionCheck */) so the real transport/auth error
is reported; keep using makePlatformLog(emit,'runtime-version') to log the
attempt and only call describeIncompatibleRuntime(versionCheck.version) after
confirming a valid version exists.
Comment on lines
+471
to
+523
| emit({ stage: 'core-install', message: 'Installing Arduino core...', level: 'info' }) | ||
| const coreInstall = await port.installArduinoCore( | ||
| { coreId: typeof boardEntry.platform === 'string' ? deriveArduinoCoreFromPlatform(boardEntry.platform) : '' }, | ||
| makePlatformLog(emit, 'core-install'), | ||
| ) | ||
| if (!coreInstall.ok) { | ||
| return bailError(emit, 'core-install', 'Failed to install Arduino core.', coreInstall.errors) | ||
| } | ||
|
|
||
| emit({ stage: 'lib-install', message: 'Installing Arduino libraries...', level: 'info' }) | ||
| const libInstall = await port.installArduinoLib({ libId: '' }, makePlatformLog(emit, 'lib-install')) | ||
| if (!libInstall.ok) { | ||
| return bailError(emit, 'lib-install', 'Failed to install Arduino libraries.', libInstall.errors) | ||
| } | ||
|
|
||
| // Build defines.h using the shared content authoring step. | ||
| const definesH = generateDefinesContent({ | ||
| boardEntry, | ||
| devicePinMapping, | ||
| stProgramFileContent: programSt, | ||
| buildMD5Hash: md5, | ||
| boardRuntime, | ||
| }) | ||
|
|
||
| // Compose firmware bundle (firmware skeleton + strucpp output + | ||
| // c_blocks header/code + defines.h). Pure function. | ||
| emit({ stage: 'firmware-bundle', message: 'Composing firmware bundle...', level: 'info' }) | ||
| const cBlocks = buildCBlocksFromPous(originalCppPous as never) | ||
| const firmwareFiles = composeFirmwareBundle({ | ||
| strucppFiles: strucppFilesMap, | ||
| cBlocks, | ||
| definesH, | ||
| firmwareSkeleton, | ||
| }) | ||
|
|
||
| // Build arduino-cli argv via the shared helper. Same input/output | ||
| // on both platforms. `boardEntry` carries `platform` / `core` / | ||
| // `c_flags` / etc. straight from `hals.json`. | ||
| const arduinoArgs = buildArduinoCliCompileArgs(boardEntry, { | ||
| sketchPath: 'examples/Baremetal/Baremetal.ino', | ||
| libraryPath: 'src', | ||
| avrLibStdCppInclude, | ||
| parallel: arduinoCliParallel, | ||
| }) | ||
|
|
||
| // --------------------------------------------------------------------- | ||
| // Step 4b (cont.): Runtime v3 branch is a sub-case that runs BEFORE | ||
| // the arduino-cli compile — it embeds C blocks into program.st and | ||
| // uploads the merged ST file directly to the device. | ||
| // --------------------------------------------------------------------- | ||
| if (isRuntimeV3) { | ||
| if (compileOnly) { | ||
| emit({ stage: 'done', message: 'Compile only mode — skipping upload to runtime v3.', level: 'info' }) |
Contributor
There was a problem hiding this comment.
Short-circuit the runtime-v3 branch before Arduino-only setup.
The v3 path currently runs installArduinoCore(), installArduinoLib(), and bundle/defines work before checking isRuntimeV3. That means a runtime-v3 upload can now fail on Arduino tooling setup even though this branch never uses compileArduino().
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/backend/shared/compile/pipeline.ts` around lines 471 - 523, The
runtime-v3 path is executed after Arduino-only setup
(installArduinoCore/installArduinoLib and bundle/defines creation) causing
unnecessary failures; modify the control flow so you check isRuntimeV3 before
calling port.installArduinoCore, port.installArduinoLib, generateDefinesContent,
composeFirmwareBundle and buildArduinoCliCompileArgs and short-circuit to the
runtime-v3 logic (respecting compileOnly and using emit/bailError as needed) so
runtime-v3 uploads never depend on Arduino tool setup; locate the relevant
symbols isRuntimeV3, installArduinoCore, installArduinoLib,
generateDefinesContent, composeFirmwareBundle, buildArduinoCliCompileArgs,
compileOnly, emit and bailError and move or guard those calls accordingly.
Comment on lines
+33
to
+36
| import type { CppPouData as CppPouDataCode } from '../../utils/cpp/generateCBlocksCode' | ||
| import { generateCBlocksCode } from '../../utils/cpp/generateCBlocksCode' | ||
| import type { CppPouData as CppPouDataHeader } from '../../utils/cpp/generateCBlocksHeader' | ||
| import { generateCBlocksHeader } from '../../utils/cpp/generateCBlocksHeader' |
Contributor
There was a problem hiding this comment.
Use @root/* imports for src modules.
Please switch these relative imports to the repository alias to match project conventions.
Suggested diff
-import type { CppPouData as CppPouDataCode } from '../../utils/cpp/generateCBlocksCode'
-import { generateCBlocksCode } from '../../utils/cpp/generateCBlocksCode'
-import type { CppPouData as CppPouDataHeader } from '../../utils/cpp/generateCBlocksHeader'
-import { generateCBlocksHeader } from '../../utils/cpp/generateCBlocksHeader'
+import type { CppPouData as CppPouDataCode } from '`@root/backend/shared/utils/cpp/generateCBlocksCode`'
+import { generateCBlocksCode } from '`@root/backend/shared/utils/cpp/generateCBlocksCode`'
+import type { CppPouData as CppPouDataHeader } from '`@root/backend/shared/utils/cpp/generateCBlocksHeader`'
+import { generateCBlocksHeader } from '`@root/backend/shared/utils/cpp/generateCBlocksHeader`'As per coding guidelines, **/*.{ts,tsx,js,jsx}: Use path alias @root/* to reference ./src/*.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/backend/shared/compile/steps/compose-firmware-bundle.ts` around lines 33
- 36, The imports in compose-firmware-bundle.ts use relative paths; update them
to the project path alias by replacing '../../utils/cpp/generateCBlocksCode' and
'../../utils/cpp/generateCBlocksHeader' with
'`@root/utils/cpp/generateCBlocksCode`' and
'`@root/utils/cpp/generateCBlocksHeader`' respectively so that the named symbols
(generateCBlocksCode, generateCBlocksHeader, and the CppPouData types aliased as
CppPouDataCode/CppPouDataHeader) are imported via the `@root/`* alias consistent
with project conventions.
Comment on lines
+92
to
+95
| // Editor's behaviour: leave the static `c_blocks.h` baseline | ||
| // in place (`null` here means the composer skips the write). | ||
| // Static `c_blocks_code.cpp` likewise stays untouched. | ||
| return { header: '// Empty file\n', code: null } |
Contributor
There was a problem hiding this comment.
Fix stale sentinel comment in empty-POU path.
This comment says null skips c_blocks.h, but src/c_blocks.h is always overwritten later (Line 148). Only c_blocks_code.cpp is conditionally skipped.
Suggested diff
- // Editor's behaviour: leave the static `c_blocks.h` baseline
- // in place (`null` here means the composer skips the write).
- // Static `c_blocks_code.cpp` likewise stays untouched.
+ // Empty-POU sentinel:
+ // - `header` is the canonical empty stub and will overwrite `src/c_blocks.h`.
+ // - `code: null` tells the composer to keep skeleton
+ // `examples/Baremetal/c_blocks_code.cpp` untouched.📝 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
| // Editor's behaviour: leave the static `c_blocks.h` baseline | |
| // in place (`null` here means the composer skips the write). | |
| // Static `c_blocks_code.cpp` likewise stays untouched. | |
| return { header: '// Empty file\n', code: null } | |
| // Empty-POU sentinel: | |
| // - `header` is the canonical empty stub and will overwrite `src/c_blocks.h`. | |
| // - `code: null` tells the composer to keep skeleton | |
| // `examples/Baremetal/c_blocks_code.cpp` untouched. | |
| return { header: '// Empty file\n', code: null } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/backend/shared/compile/steps/compose-firmware-bundle.ts` around lines 92
- 95, The comment above the empty-POU return is stale: it claims `null` for
`code` means the composer skips writing `c_blocks.h`, but `src/c_blocks.h` is
actually always overwritten later while only `c_blocks_code.cpp` is
conditionally skipped. Update the comment near the return { header: '// Empty
file\n', code: null } in compose-firmware-bundle.ts to accurately state that the
header (`c_blocks.h`) is written regardless and that only the `code` field
controls skipping of `c_blocks_code.cpp`; ensure references to `header` and
`code` reflect that behavior.
P3c-1 of the shared-build-pipeline refactor. Adds the editor's
implementation of `CompilerPlatformPort` that wraps existing
handlers (`handleTranspileXMLtoST`, `handleCompileArduinoProgram`,
`handleCoreInstallation`, `handleLibraryInstallation`,
`handleUploadProgram`, `sendRuntimeUpload`, `compressSourceFolder`,
`fetchRuntimeVersion`) behind the canonical port interface.
No behaviour change: the new module is not yet called from
`compileProgram`. The actual wiring (rewriting `compileProgram`
to use `runCompilePipeline` instead of its inline 800-LOC
orchestration, plus deleting the now-dead handlers) lands in
P3c-2 — kept as a separate commit because that's the surgical
step where regressions are likely and a focused review window
matters most.
What the adapter does:
- `computeMd5` — Node `crypto.createHash('md5')`
- `transpileXmlToSt` — materialise XML to disk, call existing
handler (which spawns xml2st reading from disk), read
`program.st` back into memory.
- `compileArduino` — materialise the in-memory file map to disk
under `compilationPath`, walk the build/<fqbn>/ tree for the
produced `.hex`, read it back as a `Uint8Array`.
- `installArduinoCore` / `installArduinoLib` — direct passthrough
to existing handlers (modulo log-shape translation).
- `uploadRuntimeV4` — materialise bundle to disk, zip via
`compressSourceFolder`, drive `deployRuntimeProgram` with
`sendRuntimeUpload` + `/api/compilation-status` poll +
`/api/start-plc` start.
- `uploadArduinoBoard` — passthrough to `handleUploadProgram`.
- `uploadRuntimeV3` — convert `program.st` to a Buffer, drive
the same `deployRuntimeProgram` flow with `text/plain` content
type. V3 is end-of-life; web's adapter will no-op this.
- `checkRuntimeVersion` — unauthenticated probe of
`/api/version` for the strucpp-compatibility gate.
`PlatformDeviceContext` discriminator narrowed to `editor-https`
via an explicit runtime check — passing a `web-orchestrator`
context to the editor adapter throws cleanly.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds `CompilerModule.loadFirmwareSkeletonInMemory(boardRuntime)`:
walks the on-disk firmware skeleton (`resources/sources/arduino/*`
+ `resources/sources/Baremetal/**`) and returns it as the
`Record<string, string>` shape the shared `composeFirmwareBundle`
takes as input.
No behaviour change: not yet called from anywhere. Adding it as
a separate commit so the eventual `compileProgram` rewrite (P3c-2,
where the wiring actually happens) has the helper it needs without
having to introduce both at once — the helper is mechanical
boilerplate and reviewable on its own.
Path mapping (matches `copyStaticFiles`'s on-disk layout exactly):
- `resources/sources/arduino/<file>` → `src/<file>`
- `resources/sources/Baremetal/<file>` → `examples/Baremetal/<file>`
- `resources/sources/Baremetal/modules/<file>` →
`examples/Baremetal/modules/<file>`
Runtime v4 (`boardRuntime === 'openplc-compiler'`) returns `{}`
because the v4 bundle is composed by `composeRuntimeV4Bundle` from
a different source (`loadStrucppRuntimeHeaders`) — no Arduino
skeleton is part of the v4 upload.
Strucpp runtime headers stay in `loadStrucppRuntimeHeaders` (their
on-disk source is `node_modules/strucpp/...`, and the runtime v4
composer wants them under `strucpp_runtime/include/<filename>`,
not `src/`).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
P3c-2 of the shared-build-pipeline refactor. Editor's
`compileProgram` (~800 LOC of Step 0-13 orchestration) collapses
to a ~250 LOC wrapper that:
1. Parses args (same shape as before)
2. Runs editor-specific preamble: project header, host hardware
info, VPP feature warnings (Modbus servers / Remote IO on
non-v4 targets), tool availability check
3. Creates the build/<target>/{src,examples/Baremetal,...} dir
tree so port methods that write to disk have somewhere to land
4. Resolves pipeline inputs through editor helpers:
- `firmwareSkeleton` via the new
`loadFirmwareSkeletonInMemory(boardRuntime)`
- `strucppRuntimeHeaders` via `loadStrucppRuntimeHeaders()`
(only for runtime v4; empty `{}` otherwise)
- `devicePinMapping` from `devices/pin-mapping.json`
- `libraryArchives` + `missingLibraries` from
`mainProcessBridge.loadEnabledArchives()`
- `avrLibStdCppInclude` via `ensureAvrLibStdCppCache()` when
the board's `core` starts with `arduino:avr`
5. Instantiates `EditorCompilerPlatformPort` with bound method
references for the editor-specific transports (xml2st spawn,
arduino-cli core+lib install, arduino-cli compile, arduino-cli
upload, runtime HTTPS upload via `sendRuntimeUpload` +
`compressSourceFolder`)
6. Builds the `PlatformDeviceContext` (`editor-https` kind) when
`runtimeIpAddress` + `runtimeJwtToken` are present
7. Calls `runCompilePipeline(args, port, emit)` — emit forwards
`PipelineProgressEvent` to `_mainProcessPort.postMessage(...)`
with `logLevel: event.level`
8. Editor-specific epilogue:
- Simulator success: emit `simulatorFirmwarePath` (resolved
per-FQBN sub-dir) + `closePort: true`
- Simulator + compileOnly: emit `'Compilation successful.'` +
`closePort: true`
- All other paths: emit the canonical
`--------...---\n` separator + deferred 25ms `closePort`
(matches the pre-refactor renderer-side timing)
Imports cleaned up: removed `describeIncompatibleRuntime`,
`isStrucppCompatibleRuntime`, `deployRuntimeProgram`,
`generateRuntimeConfs`, `parsePlcStatus`, `composeRuntimeV4Bundle`
from compiler-module.ts — all used to be called inline from the
old `compileProgram` body, now lifted into the shared pipeline.
Added `runCompilePipeline` (shared) and
`createEditorCompilerPlatformPort` (the wrapper from P3c-1).
The existing low-level handlers (`handleTranspileXMLtoST`,
`handleCompileArduinoProgram`, `handleUploadProgram`,
`handleCoreInstallation`, `handleLibraryInstallation`,
`sendRuntimeUpload`, `compressSourceFolder`,
`fetchRuntimeVersion`) all stay in `CompilerModule` — they're now
invoked via the platform port adapter rather than directly. Other
entry points (`compileForDebugger`, `compileLibrary`,
`runVerificationCompile`) still call them directly; those paths
remain on the legacy orchestration and are migration targets for
a follow-up PR.
Dead-code removal of handlers that are now solely unused will land
in a separate cleanup pass once `compileForDebugger` and
`compileLibrary` are also migrated.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/backend/editor/compiler/compiler-module.ts`:
- Around line 1927-1985: The new runCompilePipeline path omits invoking
handleVendorPluginPackaging and therefore never injects VPP plugin files into
the runtime-v4 bundle, causing uploaded zips to miss conf/<plugin>.json,
vpp_plugins.conf, and vpp_plugin/**; fix by calling handleVendorPluginPackaging
(the same step used by the old compileProgram path) before calling
platformPort.uploadRuntimeV4 (createEditorCompilerPlatformPort.uploadRuntimeV4)
and merge its outputs into the args.bundle passed to uploadRuntimeV4 so the
runtime-v4 bundle includes the VPP files; ensure this happens when boardRuntime
indicates runtime-v4 and for installed VPP boards, and reuse existing helper
logic around handleVendorPluginPackaging to produce the plugin files and updated
bundle.
In `@src/backend/editor/compiler/editor-compiler-platform-port.ts`:
- Around line 220-252: In compileArduino (in editor-compiler-platform-port.ts)
you need to invoke the existing Arduino compile handler before scanning for the
produced .hex: after materialising args.files but before calling
findHexInCompilationPath, call handleCompileArduinoProgram with the appropriate
context/args (including threading boardHalsContent through
EditorCompilerPlatformPortContext from compiler-module.ts), await its
completion, then read the .hex; ensure the handler receives the canonical
argv/boardHalsContent so a clean tree actually builds and you don't return stale
binaries.
🪄 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: 2e82cf9e-7066-49ba-9426-deaab7489593
📒 Files selected for processing (2)
src/backend/editor/compiler/compiler-module.tssrc/backend/editor/compiler/editor-compiler-platform-port.ts
User-reported: compile hangs after the pipeline's first "Preprocessing
POUs..." log line on the simulator path. Root causes were three
bugs in the editor's CompilerPlatformPort adapter (P3c-1):
1. `.call(undefined as never, ...)` was overriding the `this` binding
the editor sets up via `.bind(this)` on every handler. When the
port adapter then invoked the bound method through `.call`,
`this` came in as `undefined` — every `this.#executeXml2st(...)`
/ `this.arduinoCliBaseParameters` / etc. inside the existing
handlers crashed silently with `Cannot read properties of
undefined`. Removed the `.call` wrapper on all four affected
methods (`handleTranspileXMLtoST`, `handleCoreInstallation`,
`handleLibraryInstallation`, `handleUploadProgram`) — the
handlers come in pre-bound, so a direct invocation is correct.
2. `compileArduino` materialised the in-memory file map to disk but
never actually invoked `handleCompileArduinoProgram`. It then
tried to find the `.hex` arduino-cli would have produced and
crashed because no compile had run. Added the missing call with
the right arg shape.
3. `boardHalsContent` (the per-board entry from `hals.json` —
`platform`, `c_flags`, `cxx_flags`, `max_data_size`, etc.) wasn't
plumbed through the port adapter's context, so `compileArduino`
had no way to pass it to `handleCompileArduinoProgram`. Added
to `EditorCompilerPlatformPortContext` and wired through
`compileProgram`.
Plus a guard rail in the shared pipeline itself:
4. `runCompilePipeline` now wraps its main body in a try/catch.
Any unhandled throw inside the orchestrator (data shape
mismatch, port impl crash, strucpp module load failure) surfaces
as a single canonical error event ('Unhandled pipeline error:
<message>') followed by the 'Stopping compilation process.'
trailer the renderer already understands — instead of leaving
the IPC channel hanging waiting on a success/failure event that
never arrives.
Together these fixes resolve the simulator-path hang. Other
paths (runtime v4, arduino-direct, runtime v3) used the same
`.call`-broken handler invocations and benefit from the same fix.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
User-reported: pipeline crashes inside `preprocessPous` reading
`pou.body.language` because `pou.body` is undefined.
Root cause: each platform's renderer ALREADY preprocesses POUs
(Python → ST stub, C/C++ → ST stub + `originalCppPous` sidecar)
before the pipeline runs:
- Editor: the compile-action in the renderer preprocesses the
store state, then posts the IPC message that ultimately
invokes `compileProgram` in main process. The renderer's
"Found Python POU: ..." log shows in the user's compile output
BEFORE "Starting compilation process..." — that's the
renderer-side preprocess.
- Web: same — `compileProgram` in `compiler-adapter.ts`
preprocesses before invoking any compile work.
The shared pipeline calling `preprocessPous` again meant
double-processing, AND on editor specifically it crashed because
the IPC layer converts POUs from the renderer's port-shape
(`pou.body`) into the main-process schema-shape (`pou.data.body`)
on the way through. `preprocessPous`'s port-shape assumptions
then read `pou.body.language` and tripped on `undefined`.
Fix: the pipeline trusts that `projectData.pous` are already in
ST form and `originalCppPous` is already attached when the project
has C/C++ POUs. Drops the `preprocessPous` call + the
`Step 0: Preprocess POUs` emit + the now-orphaned `preprocessLogger`.
Both platforms preprocess upstream of the pipeline.
Also removes the obsolete preprocess-validation test case and its
mock setup (82 tests now, was 83 — one test removed).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
User-reported: arduino-cli compile fails with
fatal error: debug_dispatch.hpp: No such file or directory
when building for the simulator (or any Arduino target).
`debug_dispatch.hpp` / `iec_std_lib.hpp` / etc. are strucpp runtime
headers. They live in two different layouts on disk depending on
the target:
- Runtime v4: `strucpp_runtime/include/<filename>` (the key
`composeRuntimeV4Bundle` expects).
- Arduino / simulator: flat under `src/<filename>` next to the
strucpp-generated artefacts, matching what editor's pre-refactor
`copyStrucppRuntimeHeaders(sourceTargetFolderPath)` materialised.
The pipeline was loading these headers ONLY for runtime v4 (the
v4-targeted layout) and passing `{}` for Arduino/simulator builds.
`ModbusSlave.cpp` includes `"debug_dispatch.hpp"` — arduino-cli's
`--library src` pass couldn't resolve it because no such file lived
under `src/`.
Fix: editor's `compileProgram` now merges the strucpp runtime
headers into the firmware skeleton at `src/<filename>` for
non-v4 targets, in addition to passing them through at the v4
layout for v4 builds. This restores the same on-disk shape
arduino-cli sees as in the pre-refactor flow.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
User-reported: link step fails with undefined reference to `hardwareInit` undefined reference to `updateInputBuffers` undefined reference to `updateOutputBuffers` These functions are defined in the board-specific HAL adapter (`resources/sources/hal/<boardEntry.source>` — e.g. `mega_due.cpp` for the simulator). Editor's pre-refactor pipeline ran `handleGenerateArduinoCppFile` (Step 11), which copied that HAL to `build/<boardTarget>/src/arduino.cpp` so the linker could resolve the symbols. My refactor skipped Step 11 because the firmware skeleton composer covers the Arduino-side files — but the per-board HAL adapter wasn't part of the skeleton I loaded (it lives under a different directory, `resources/sources/hal/`, and is selected by `boardEntry.source` rather than walked from a fixed path). Fix: editor's `compileProgram` now reads `resources/sources/hal/<boardEntry.source>` and adds it to the firmware skeleton at `src/arduino.cpp` for non-v4 targets. Matches the editor's pre-refactor on-disk shape exactly. If the HAL file is missing or unreadable, a warning is emitted through the IPC and the compile proceeds (the link will still fail, but the user gets a clearer diagnostic than "undefined reference"). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Arduino.h defines `min` and `max` as preprocessor macros, which collide with the `std::min` / `std::max` function templates and `numeric_limits<T>::min()` / `max()` static members declared by <algorithm> / <limits> (both pulled in transitively via iec_string.hpp). Without scrubbing, projects with a C/C++ POU fail to link with "macro min requires 2 arguments, but only 1 given" cascades across the entire AVR libstdc++ tree. Undef both macros immediately after `#include <Arduino.h>` and before the strucpp runtime headers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…path Refactor dropped the call to handleVendorPluginPackaging, so v4 boards shipped a generic Runtime v4 upload — no plugin config under conf/, no vpp_plugins.conf to enable the driver, no vpp_plugin/ source upload or checksum. Programs uploaded fine but physical I/O was dead because the driver was never loaded. Add `packageVppPlugin` to CompilerPlatformPort. Pipeline invokes it unconditionally on the v4 branch between composeRuntimeV4Bundle and uploadRuntimeV4 — mirrors the pre-refactor ordering. Editor's adapter wraps the existing handler (self-gates on "is this board from a VPP package?", writes to sourceTargetFolderPath; compressSourceFolder picks up the writes). Returned files map merges into the bundle so non-zip transports (web) can carry VPP files in-bundle later; editor returns an empty map and lets the disk layer be the source of truth. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
openplc-runtime's build_state.log() appends "\n" to every entry it pushes (see webserver/plcapp_management.py). The classifier anchored the body match with `$`, which JS — without the `m` flag — refuses to match before a trailing newline. Every line bailed out of the regex match and routed as level='info' with the [LEVEL] prefix preserved, so the console rendered errors blue. Anchor only the prefix and slice the rest, stripping any trailing CRLF before forwarding. Regression test pins the four prefix variants with explicit "\n" terminators. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tions, tests Address review feedback on PR #794: Issue #4 — findHexInCompilationPath picked the first FQBN sub-dir, returning the wrong binary when a stale build from another board lived alongside the current one (compile for Mega, switch to Uno, upload → arduino-cli flashed the Mega hex). Helper now takes the expected FQBN and constructs the canonical path (`examples/Baremetal/build/<fqbn-with-dots>/Baremetal.ino.hex`) — same derivation the simulator branch in compileProgram has always used. The directory walk stays as a safety fallback for cores that mangle the FQBN through aliases. Issue #5 — uploadArduinoBoard ignored args.port and let `handleUploadProgram` re-read the value from `devices/configuration.json` on disk, which lags the live UI state by a save round-trip. Plumb the serial port through explicitly: renderer (workspace-activity-bar) → CompileProgramArgs (communicationPort) → IPC bridge → compileProgram entrypoint → runCompilePipeline (new RunCompilePipelineArgs.communicationPort) → port.uploadArduinoBoard → handleUploadProgram. The handler still falls back to its disk read when the arg is absent (compileForDebug and other legacy callers). Issue #9 — extract two pure helpers from compileProgram to shared: - `resolveBoardSelection(halsContent, boardTarget)` — boardEntry lookup + the four mutually-exclusive runtime flags (isSimulator/isRuntimeV3/isRuntimeV4/Arduino-direct). Was duplicated character-for-character between editor and web. - `mergeStrucppRuntimeIntoSkeleton({skeleton, runtimeHeaders, boardHalContent})` — re-key strucpp runtime headers from the v4 `strucpp_runtime/include/<file>` layout into `src/<file>` for the Arduino-cli firmware build, plus drop the board HAL at `src/arduino.cpp` when supplied. Same merge editor's compileProgram did inline; pulled to shared so both repos can call it. Tests — fill the gaps the reviewer flagged: - `resolve-board-selection.test.ts` (7 tests) + `merge-strucpp- runtime-into-skeleton.test.ts` (7 tests) covering the new shared helpers. - `editor-compiler-platform-port.test.ts` (22 tests) covering assertEditorHttpsContext, findHexInCompilationPath (incl. the stale-FQBN regression scenario for #4), and the port methods' handler-error mapping + log-callback Buffer→string coercion. - `load-firmware-skeleton.test.ts` (8 tests) pinning the arduino/* → src/*, Baremetal/* → examples/Baremetal/*, and Baremetal/modules/* → examples/Baremetal/modules/* path rules, plus the "openplc-compiler short-circuit returns {}" branch and the defensive error-swallow behaviour when one or both source dirs are missing. - `pipeline.test.ts` — replace the non-asserting EtherCAT test (`expect(typeof result.success).toBe('boolean')`) with a deterministic mock of `generateRuntimeConfs` throwing, then assert the pipeline returns success=false, surfaces the validator's message through an error event, and skips the upload step. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous PR-feedback commit extracted `resolveBoardSelection` to
shared, but the helper only checks `hals.json` — it doesn't know
about installed VPP packages, which is where boards like SLM-RP4
live. The pre-refactor `#getBoardRuntime` had the fallback:
1. Look up boardTarget in hals.json
2. If absent, scan `packageManager.listInstalled()` for a device
whose `name === boardTarget` and derive the runtime from
`device.target.type`
3. Throw only if neither matched
The refactored code took only step 1, so VPP boards regressed with
`hals.json is missing the "SLM-RP4" entry — bundled asset is out
of sync.`
Restore the fallback in editor's compileProgram. The shared helper
stays pure (hals-only) — web has no installed-package surface, so
the fallback is editor-specific and stays here. VPP boards don't
ship a hals.json entry; the pipeline gets an empty placeholder for
`boardEntry` (the runtime-v4 branch, which is the canonical target
for VPP, doesn't dereference its arduino-cli-specific fields).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sync of openplc-web PR #431's coverage push so the byte-identical shared test file stays in lockstep. Adds: - uploadArduinoBoard reports failure → pipeline returns success=false - VPP merge log line ("Merged N VPP plugin file(s) into bundle") - The `instances.map(inst => ({...}))` lambda in the v4 confs invocation - generate-confs's `log` callback forwarding to the emit channel - Outer try/catch wrapping unhandled Error + non-Error throws - PlatformLog callback execution via transpileXmlToSt log lines Brings pipeline.ts coverage to 100% statements/lines/functions on the openplc-web vitest run (was 99.26/100/92.3). Editor's jest run still passes 41/41. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…sion
Two structural cleanups requested in the PR review thread:
# hals.json now lives in the shared backend surface
The canonical board catalogue moves from
`resources/sources/boards/hals.json` to
`src/backend/shared/firmware/hals.json` so the Shared Surface Sync
CI gate enforces byte-identical content between editor and web.
Editor's `compiler-module.ts` and `hardware-module.ts` consumers
now go through a small `readHalsFile()` helper that wraps the
bundled JSON in a Promise — keeps the existing async call sites
unchanged. `#constructHalsFilePath()` + the `halsFilePath` field
on `CompilerModule` are dropped; the old `resources/` copy is
deleted.
# Response-parse + null-fallback logic for the runtime version
# probe moves into shared `probeRuntimeVersion`
Editor and web were each implementing identical response handling
("got a body, extract `version`, fall back to null on any failure,
log a warning") on top of platform-specific transports. New
`backend/shared/library/probe-runtime-version.ts` owns the shared
half: it takes a transport callback returning
`{success, body | error}` and produces the canonical
`{version: string | null}` the strucpp-compatibility gate consumes.
Editor's `checkRuntimeVersion` shrinks to a 6-line shim that wraps
its HTTPS bridge in the new envelope shape and hands it to
`probeRuntimeVersion`. 9 dedicated tests cover the helper's
branches (happy path, transport failure, sync throw, non-Error
throw, missing field, non-string version, null body, primitive
body).
Web side mirrors the change in PR #431 — adapter passes the raw
body envelope through; shared helper does the parse.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CI lint job rejected the import block on hardware-module.ts after the `readHalsFile` import landed in commit 6a71053 — the simple-import-sort plugin wants node builtins, then external packages, then internal imports, each block separated by a blank line. Autofix sorts them cleanly; no behaviour change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Code-style only — no behavior change. CI's `format` gate was failing on these files; running prettier --write resolves it without touching logic. Shared files (`backend/shared/compile/**`, `backend/shared/library/ __tests__/probe-runtime-version.test.ts`, `backend/shared/utils/cpp/ __tests__/generateCBlocksCode.test.ts`) will be sync-copied to openplc- web in the parallel PR so the Shared Surface Sync gate stays green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two pre-existing shared-surface drifts where openplc-web had small runtime improvements that hadn't been mirrored back to the editor. Bringing the editor up to match unblocks the Shared Surface Sync gate on PR #794 (the refactor PR that finally lit up sync after upstream lint/build/format started passing). - `AcuExhaustionModal.tsx`: wrap `monthlyLimit` / `required` / `remaining` in `Math.round(...)` so a fractional ACU balance from the billing API doesn't render as e.g. "47.999 ACU". Web behaviour, ported as-is. - `monaco/index.tsx`: extract `inlineCompletionsActive` (gates `capabilities.hasAIAssistant` on `aiState.isEnabled` + `hasConsented` + `preferences.inlineCompletionsEnabled`) and use it for `quickSuggestions` + the `inlineCompletionsProvider` install. Restores Monaco's auto-popup when the user turns inline completions off — the prior `capabilities.hasAIAssistant`-only gate suppressed it unconditionally. Web behaviour, ported as-is. No logic delta from web's current `development`; files copied verbatim so the byte-identical invariant holds. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ck json-first resolution Bug: empty board-settings screen on the editor (no built-in boards, no VPP devices) after the hals.json move to shared. Root cause: editor's webpack `resolve.extensions` lists `['.js', '.jsx', '.json', '.ts', '.tsx']` — `.json` before `.ts`. Both `compiler-module.ts` and `hardware-module.ts` imported the new shared loader as `'@root/backend/shared/firmware/hals'` (extensionless), which webpack happily resolved to the *data* file `hals.json` sitting alongside the `hals.ts` loader. The imported namespace was the parsed board catalogue object, so `readHalsFile()` was reading a missing property off it and ending up `undefined` — every call site then threw at runtime and `getAvailableBoards()`'s exception got swallowed by the IPC bridge, leaving the renderer with an empty Map. Could have re-ordered `resolve.extensions` instead, but tooling re-orders ripple through every other extensionless import in the codebase. Renaming the loader keeps the change local to two files on each repo, leaves the data file's name (`hals.json`) as the canonical reference everyone already knows about, and is robust against both webpack and Vite resolution rules. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
marconetsf
added a commit
that referenced
this pull request
May 29, 2026
Arduino.h defines `min` and `max` as preprocessor macros, which collide with the `std::min` / `std::max` function templates and `numeric_limits<T>::min()` / `max()` static members declared by <algorithm> / <limits> (both pulled in transitively via iec_string.hpp). Without scrubbing, projects with a C/C++ POU fail to build with "macro min requires 2 arguments, but only 1 given" cascades across the entire AVR libstdc++ tree. Undef both macros immediately after `#include <Arduino.h>` and before the strucpp runtime headers. Back-port of 6a5fbf6 (already on origin/development via #794) — this branch (feat/vpp-compile-pipeline-port) diverged at b181234 before that fix landed, so AVR projects with a C/C++ POU were still broken here. The previous commit on this branch (which makes the precompile actually find Arduino.h via the -I{core,variant} fix) is what surfaces this collision in the first place. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Jun 4, 2026
This was referenced Jun 18, 2026
Merged
This was referenced Aug 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Migrates the OpenPLC build pipeline into a single shared module driven through a thin
CompilerPlatformPortadapter. Editor and (in a follow-up PR) openplc-web will both compile through the same Step 0-13 orchestrator; only the three genuinely platform-specific calls — xml2st transpile, arduino-cli compile, runtime upload — sit behind adapter methods.Editor's
compileProgramshrinks from ~800 LOC of inline orchestration to ~250 LOC of thin wrapping aroundrunCompilePipeline.Commits
refactor(compile): extract defines.h authoring into sharedrefactor(compile): extract runtime-v4 conf orchestration into shared(also deletes 5 dead handlers)feat(compile): add CompilerPlatformPort + composeFirmwareBundlefeat(compile): shared pipeline orchestrator(the heart —runCompilePipeline+ 32 tests covering all four paths)feat(compile): editor implementation of CompilerPlatformPortfeat(compile): firmware-skeleton-in-memory loaderfeat(compile): editor compileProgram uses shared pipelineWhat the shared pipeline does
runCompilePipeline(args, port, emit)drives the four editor-canonical paths:uploadArduinoBoardcompileOnlyshort-circuits each path before the upload step.Editor adoption
Editor's
compileProgram:EditorCompilerPlatformPortvia the wrapper ateditor-compiler-platform-port.ts(binds existing handlers behind the port contract)runCompilePipeline—emitforwards events to_mainProcessPort.postMessageclosePort)Tests
npx tsc --noEmit -p tsconfig.jsonpassesnpx eslintclean on all touched filesdevelopment.hex(simulator) and v4 upload.zipagainst pre-refactor outputs for a fixed test projectOut of scope (follow-up PRs)
compileForDebuggerandcompileLibrarystill use their inline orchestration. Migrating them torunCompilePipeline(with amodeflag) is a separate PR. Existing low-level handlers (handleTranspileXMLtoST,handleCompileArduinoProgram, etc.) stay inCompilerModulefor now because they're still called by those paths and by the new platform port adapter.compileForDebugger/compileLibrarymigrate.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Tests