From 350955ab7a16400b83d6539a7cb3106ace50baa0 Mon Sep 17 00:00:00 2001 From: Jens Geudens Date: Thu, 2 Apr 2026 19:18:10 +0200 Subject: [PATCH 1/6] Add sub agents --- .claude/agents/build.md | 15 +++++++++++++++ .claude/agents/quality.md | 17 +++++++++++++++++ .claude/agents/test-runner.md | 10 ++++++++++ CLAUDE.md | 11 ++++++++--- 4 files changed, 50 insertions(+), 3 deletions(-) create mode 100644 .claude/agents/build.md create mode 100644 .claude/agents/quality.md create mode 100644 .claude/agents/test-runner.md diff --git a/.claude/agents/build.md b/.claude/agents/build.md new file mode 100644 index 00000000..3c36e89d --- /dev/null +++ b/.claude/agents/build.md @@ -0,0 +1,15 @@ +--- +name: build +description: Compile the project using cmake and ninja. Use when code changes need to be built or when checking for compilation errors. +tools: Bash +--- + +Build the project from the project root. + +If the build directory doesn't exist yet, run: + mkdir -p build && cmake -GNinja -S . -B build + +Then always run: + ninja -C build + +Report only errors and warnings. If the build succeeds, say so briefly. diff --git a/.claude/agents/quality.md b/.claude/agents/quality.md new file mode 100644 index 00000000..6ae0cd92 --- /dev/null +++ b/.claude/agents/quality.md @@ -0,0 +1,17 @@ +--- +name: quality +description: Run pre-commit quality checks (clang-format, clang-tidy, clazy) on source files. Use after making code changes before finishing a task. +tools: Bash +--- + +Run quality checks from the project root. + +For a specific file (faster): + clang-format -i + ./scripts/run_clang_tidy.sh + ./scripts/run_clazy.sh + +For all files (slow): + ./scripts/run_precommit.sh + +Report only violations with file, line, and error message. If all checks pass, say so briefly. diff --git a/.claude/agents/test-runner.md b/.claude/agents/test-runner.md new file mode 100644 index 00000000..a536d5f2 --- /dev/null +++ b/.claude/agents/test-runner.md @@ -0,0 +1,10 @@ +--- +name: test-runner +description: Run the test suite with ctest. Use after making code changes to verify correctness. The project must be built first. +tools: Bash +--- + +Run the test suite from the project root: + ctest --test-dir build --output-on-failure + +Report only failing tests with their error messages. If all tests pass, say so briefly with the count. diff --git a/CLAUDE.md b/CLAUDE.md index b8733f2f..c0f626c3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -40,7 +40,7 @@ clang-format -i src/path/to/file.cpp ```text src/ -├── communication/ Modbus protocol layer (ModbusPoll, ModbusMaster, ModbusConnection) +├── communication/ Communication (using external adapters) ├── models/ Data models (SettingsModel, GraphDataModel, DiagnosticModel, Device, Connection) ├── datahandling/ Expression parsing, graph data processing ├── importexport/ CSV export, MBS project files, MBC device config import @@ -82,5 +82,10 @@ Enforced by `.clang-format` (Mozilla-based, C++20): ## Development -- Use a subagent to run the test suite and report only the failing tests with their error messages. -- Use a subagent to run the quality checks (clang, clazy) and report only the violations with their error messages. +Three sub-agents are defined in `.claude/agents/` to keep build/test/lint output out of the main context: + +- **`@agent-build`** — runs cmake + ninja; reports only errors and warnings. +- **`@agent-test-runner`** — runs ctest; reports only failing tests with their error messages. +- **`@agent-quality`** — runs clang-format, clang-tidy, and clazy; reports only violations. + +Always use these agents rather than running the commands directly. After making source file changes: build, then run tests, then run quality checks — all must pass before the work is done. From ccc3744c871f6678a654870489bfc3bdaaef522c Mon Sep 17 00:00:00 2001 From: Jens Geudens Date: Thu, 2 Apr 2026 19:26:42 +0200 Subject: [PATCH 2/6] Specify model --- .claude/agents/build.md | 1 + .claude/agents/quality.md | 1 + .claude/agents/test-runner.md | 1 + 3 files changed, 3 insertions(+) diff --git a/.claude/agents/build.md b/.claude/agents/build.md index 3c36e89d..e814e10d 100644 --- a/.claude/agents/build.md +++ b/.claude/agents/build.md @@ -2,6 +2,7 @@ name: build description: Compile the project using cmake and ninja. Use when code changes need to be built or when checking for compilation errors. tools: Bash +model: Haiku --- Build the project from the project root. diff --git a/.claude/agents/quality.md b/.claude/agents/quality.md index 6ae0cd92..acc48bae 100644 --- a/.claude/agents/quality.md +++ b/.claude/agents/quality.md @@ -2,6 +2,7 @@ name: quality description: Run pre-commit quality checks (clang-format, clang-tidy, clazy) on source files. Use after making code changes before finishing a task. tools: Bash +model: Haiku --- Run quality checks from the project root. diff --git a/.claude/agents/test-runner.md b/.claude/agents/test-runner.md index a536d5f2..7e8b29a7 100644 --- a/.claude/agents/test-runner.md +++ b/.claude/agents/test-runner.md @@ -2,6 +2,7 @@ name: test-runner description: Run the test suite with ctest. Use after making code changes to verify correctness. The project must be built first. tools: Bash +model: Haiku --- Run the test suite from the project root: From 076a46c767957518632c9a1f0ebad869723fdcfc Mon Sep 17 00:00:00 2001 From: Jens Geudens Date: Thu, 2 Apr 2026 20:19:01 +0200 Subject: [PATCH 3/6] Add reviewer subagent --- .claude/agents/code-reviewer.md | 73 +++++++++++++++++++++++++++++++++ CLAUDE.md | 9 ++-- 2 files changed, 78 insertions(+), 4 deletions(-) create mode 100644 .claude/agents/code-reviewer.md diff --git a/.claude/agents/code-reviewer.md b/.claude/agents/code-reviewer.md new file mode 100644 index 00000000..f47103b6 --- /dev/null +++ b/.claude/agents/code-reviewer.md @@ -0,0 +1,73 @@ +--- +name: code-reviewer +description: Reviews Qt/C++ code for quality, safety, and best practices +tools: Read, Glob, Grep +model: sonnet +--- + +You are a senior Qt/C++ code reviewer. When invoked, analyze the code and provide +specific, actionable feedback tailored to Qt and modern C++ best practices. + +Evaluate: + +### Qt-specific best practices +- Correct use of signals and slots (including new-style connections) +- QObject ownership and parent-child memory management +- Threading correctness (QThread, moveToThread, signal/slot thread safety) +- UI responsiveness (avoid blocking the main thread) +- Correct use of Qt macros (Q_OBJECT, Q_PROPERTY, etc.) +- Resource management (QScopedPointer, QSharedPointer, parent ownership) + +### C++ quality +- RAII and memory safety (avoid raw `new/delete` where possible) +- Const-correctness and references +- Copy/move semantics +- Clear naming and class design + +### Correctness +- Potential bugs (null dereferences, lifetime issues, race conditions) +- Signal-slot connection errors (wrong signatures, missing connections) +- Misuse of Qt APIs + +### Performance +- Unnecessary copies (especially with QString, QVector, etc.) +- Inefficient signal emissions or event handling +- UI or thread bottlenecks + +### Security & robustness +- Input validation +- Unsafe string or file handling +- Thread-safety issues + +### Testability +- Separation of UI and logic +- Ability to unit test (e.g., avoiding tight coupling to QWidget) +- Use of dependency injection where appropriate + +--- + +Format your response as: + +## Summary +Brief overall assessment + +## Critical Issues +- [Critical] Issue description + why it matters + how to fix + +## Warnings +- [Warning] Issue description + suggested fix + +## Suggestions +- [Suggestion] Improvements or best practices + +## Qt-Specific Notes +- Qt idioms or framework-specific recommendations + +--- + +Guidelines: + +- Highlight ownership issues explicitly (who owns what) +- Call out threading mistakes clearly (they are often subtle and critical) +- Suggest concrete code improvements where helpful +- If context is incomplete, state assumptions clearly \ No newline at end of file diff --git a/CLAUDE.md b/CLAUDE.md index c0f626c3..c563953f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -84,8 +84,9 @@ Enforced by `.clang-format` (Mozilla-based, C++20): Three sub-agents are defined in `.claude/agents/` to keep build/test/lint output out of the main context: -- **`@agent-build`** — runs cmake + ninja; reports only errors and warnings. -- **`@agent-test-runner`** — runs ctest; reports only failing tests with their error messages. -- **`@agent-quality`** — runs clang-format, clang-tidy, and clazy; reports only violations. +- **`@agent-build`** - runs cmake + ninja; reports only errors and warnings. +- **`@agent-test-runner`** - runs ctest; reports only failing tests with their error messages. +- **`@agent-quality`** - runs clang-format, clang-tidy, and clazy; reports only violations. +- **`@agent-code-reviewer`** - reviews code for quality, safety, and best practices; provides specific, actionable feedback. -Always use these agents rather than running the commands directly. After making source file changes: build, then run tests, then run quality checks — all must pass before the work is done. +Always use these agents rather than running the commands directly. After making source file changes: build, then run tests, then run quality checks - all must pass before the work is done. From abc943c91df09372e0ade19087f933c99d13cb28 Mon Sep 17 00:00:00 2001 From: Jens Geudens Date: Thu, 2 Apr 2026 21:16:20 +0200 Subject: [PATCH 4/6] Fix count --- CLAUDE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index c563953f..8b9c4942 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -82,7 +82,7 @@ Enforced by `.clang-format` (Mozilla-based, C++20): ## Development -Three sub-agents are defined in `.claude/agents/` to keep build/test/lint output out of the main context: +Several sub-agents are defined in `.claude/agents/` to keep build/test/lint output out of the main context: - **`@agent-build`** - runs cmake + ninja; reports only errors and warnings. - **`@agent-test-runner`** - runs ctest; reports only failing tests with their error messages. From 63a5e1903c43ed75bce1fd8d4574da747c636f52 Mon Sep 17 00:00:00 2001 From: Jens Geudens Date: Thu, 2 Apr 2026 19:57:02 +0200 Subject: [PATCH 5/6] Refactor device settings ui --- src/dialogs/devicesettings.cpp | 31 ++++++++++--------------- src/dialogs/devicesettings.h | 8 ++----- src/dialogs/devicesettings.ui | 41 ---------------------------------- 3 files changed, 14 insertions(+), 66 deletions(-) delete mode 100644 src/dialogs/devicesettings.ui diff --git a/src/dialogs/devicesettings.cpp b/src/dialogs/devicesettings.cpp index 0ac11905..dff3391e 100644 --- a/src/dialogs/devicesettings.cpp +++ b/src/dialogs/devicesettings.cpp @@ -1,20 +1,18 @@ #include "devicesettings.h" -#include "ui_devicesettings.h" #include "customwidgets/deviceform.h" #include "models/device.h" -#include -#include +#include DeviceSettings::DeviceSettings(SettingsModel* pSettingsModel, QWidget* parent) - : QWidget(parent), _pUi(new Ui::DeviceSettings), _pSettingsModel(pSettingsModel) + : QWidget(parent), _pDeviceTabs(new AddableTabWidget(this)), _pSettingsModel(pSettingsModel) { - _pUi->setupUi(this); + QVBoxLayout* layout = new QVBoxLayout(this); + layout->addWidget(_pDeviceTabs); - connect(_pUi->deviceTabs, &AddableTabWidget::tabClosed, this, &DeviceSettings::handleCloseTab, - Qt::DirectConnection); - connect(_pUi->deviceTabs, &AddableTabWidget::addTabRequested, this, &DeviceSettings::handleAddTab); + connect(_pDeviceTabs, &AddableTabWidget::tabClosed, this, &DeviceSettings::handleCloseTab, Qt::DirectConnection); + connect(_pDeviceTabs, &AddableTabWidget::addTabRequested, this, &DeviceSettings::handleAddTab); QList pages; QStringList names; @@ -25,12 +23,7 @@ DeviceSettings::DeviceSettings(SettingsModel* pSettingsModel, QWidget* parent) pages.append(createForm(devId)); } - _pUi->deviceTabs->setTabs(pages, names); -} - -DeviceSettings::~DeviceSettings() -{ - delete _pUi; + _pDeviceTabs->setTabs(pages, names); } DeviceForm* DeviceSettings::createForm(deviceId_t devId) @@ -47,7 +40,7 @@ void DeviceSettings::handleAddTab() deviceId_t devId = _pSettingsModel->addNewDevice(); QString name = constructTabName(devId); - _pUi->deviceTabs->addNewTab(name, createForm(devId)); + _pDeviceTabs->addNewTab(name, createForm(devId)); } QString DeviceSettings::constructTabName(deviceId_t devId) @@ -59,9 +52,9 @@ QString DeviceSettings::constructTabName(deviceId_t devId) void DeviceSettings::updateTabName(deviceId_t devId) { int index = -1; - for (int i = 0; i < _pUi->deviceTabs->count(); ++i) + for (int i = 0; i < _pDeviceTabs->count(); ++i) { - auto tabContent = dynamic_cast(_pUi->deviceTabs->tabContent(i)); + auto tabContent = dynamic_cast(_pDeviceTabs->tabContent(i)); if (tabContent && tabContent->deviceId() == devId) { index = i; @@ -72,13 +65,13 @@ void DeviceSettings::updateTabName(deviceId_t devId) if (index != -1) { QString name = constructTabName(devId); - _pUi->deviceTabs->setTabName(index, name); + _pDeviceTabs->setTabName(index, name); } } void DeviceSettings::handleCloseTab(int index) { - auto tabContent = dynamic_cast(_pUi->deviceTabs->tabContent(index)); + auto tabContent = dynamic_cast(_pDeviceTabs->tabContent(index)); if (tabContent) { deviceId_t devId = tabContent->deviceId(); diff --git a/src/dialogs/devicesettings.h b/src/dialogs/devicesettings.h index acd5f352..9b172b26 100644 --- a/src/dialogs/devicesettings.h +++ b/src/dialogs/devicesettings.h @@ -1,23 +1,19 @@ #ifndef DEVICESETTINGS_H #define DEVICESETTINGS_H +#include "customwidgets/addabletabwidget.h" #include "models/settingsmodel.h" #include // Forward declaration class DeviceForm; -namespace Ui { -class DeviceSettings; -} - class DeviceSettings : public QWidget { Q_OBJECT public: explicit DeviceSettings(SettingsModel* pSettingsModel, QWidget* parent = nullptr); - ~DeviceSettings(); private slots: void handleAddTab(); @@ -28,7 +24,7 @@ private slots: DeviceForm* createForm(deviceId_t devId); QString constructTabName(deviceId_t devId); - Ui::DeviceSettings* _pUi; + AddableTabWidget* _pDeviceTabs; SettingsModel* _pSettingsModel; }; diff --git a/src/dialogs/devicesettings.ui b/src/dialogs/devicesettings.ui deleted file mode 100644 index b1f7cd30..00000000 --- a/src/dialogs/devicesettings.ui +++ /dev/null @@ -1,41 +0,0 @@ - - - DeviceSettings - - - - 0 - 0 - 400 - 300 - - - - Form - - - - - - 0 - - - - Tab 1 - - - - - - - - - AddableTabWidget - QTabWidget -
customwidgets/addabletabwidget.h
- 1 - - - - - From 05fa21f45198d4f88bcab3f6ad1b75ef96dc4df5 Mon Sep 17 00:00:00 2001 From: Jens Geudens Date: Thu, 2 Apr 2026 21:21:46 +0200 Subject: [PATCH 6/6] Review remarks --- src/dialogs/devicesettings.cpp | 5 +++-- src/dialogs/devicesettings.h | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/dialogs/devicesettings.cpp b/src/dialogs/devicesettings.cpp index dff3391e..1b4450b9 100644 --- a/src/dialogs/devicesettings.cpp +++ b/src/dialogs/devicesettings.cpp @@ -1,5 +1,6 @@ #include "devicesettings.h" +#include "customwidgets/addabletabwidget.h" #include "customwidgets/deviceform.h" #include "models/device.h" @@ -54,7 +55,7 @@ void DeviceSettings::updateTabName(deviceId_t devId) int index = -1; for (int i = 0; i < _pDeviceTabs->count(); ++i) { - auto tabContent = dynamic_cast(_pDeviceTabs->tabContent(i)); + auto tabContent = qobject_cast(_pDeviceTabs->tabContent(i)); if (tabContent && tabContent->deviceId() == devId) { index = i; @@ -71,7 +72,7 @@ void DeviceSettings::updateTabName(deviceId_t devId) void DeviceSettings::handleCloseTab(int index) { - auto tabContent = dynamic_cast(_pDeviceTabs->tabContent(index)); + auto tabContent = qobject_cast(_pDeviceTabs->tabContent(index)); if (tabContent) { deviceId_t devId = tabContent->deviceId(); diff --git a/src/dialogs/devicesettings.h b/src/dialogs/devicesettings.h index 9b172b26..13ca9ff1 100644 --- a/src/dialogs/devicesettings.h +++ b/src/dialogs/devicesettings.h @@ -1,11 +1,11 @@ #ifndef DEVICESETTINGS_H #define DEVICESETTINGS_H -#include "customwidgets/addabletabwidget.h" #include "models/settingsmodel.h" #include -// Forward declaration +// Forward declarations +class AddableTabWidget; class DeviceForm; class DeviceSettings : public QWidget