UI: platform profile picker for validation workflows (#370) - #715
Conversation
…). Expose built-in scan profiles in the inspector with QSettings persistence, run scans via an isolated qtmesh subprocess (matching CLI --target), and share profile→ScanConfig merge through buildScanConfigWithPlatformProfile. Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughThis PR introduces end-to-end asset folder scanning in Validation mode. A new ChangesAsset Folder Scanning Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
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 docstrings
🧪 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 |
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…ole. Add Browse/Open Asset Browser controls in the scan panel so the assets folder is easier to set. Co-authored-by: Cursor <cursoragent@cursor.com>
Migrate stale QSettings on picker version bump, sort Modern Console first, strip validation suffixes from combo labels, and shorten bundled profile display names. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
src/AssetScanController.cpp (1)
295-300: ⚡ Quick winSurface profile-setup warnings before launching the scan.
buildScanConfigWithPlatformProfile(...)returnssetup.warnings, but they are discarded. That hides partial profile-parse problems from users and telemetry.Suggested fix
const PlatformProfileScanSetup setup = buildScanConfigWithPlatformProfile(m_selectedProfileId); if (!setup.ok) { emit error(setup.error); emit scanFinished(false, setup.error); return; } + for (const QString& w : setup.warnings) { + SentryReporter::addBreadcrumb(QStringLiteral("ui.action"), + QStringLiteral("asset scan profile warning: %1").arg(w), + QStringLiteral("warning")); + }🤖 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/AssetScanController.cpp` around lines 295 - 300, buildScanConfigWithPlatformProfile(m_selectedProfileId) returns PlatformProfileScanSetup which includes setup.warnings but the current code discards them; update the logic in the block that checks setup.ok to first check if setup.warnings is non-empty and surface each warning (e.g., emit a warning signal and/or log it to telemetry) before emitting error/scanFinished or returning so partial profile-parse warnings are visible to users and telemetry; reference PlatformProfileScanSetup, buildScanConfigWithPlatformProfile, setup.warnings, m_selectedProfileId, and the existing emit error / emit scanFinished calls when adding the warning handling.
🤖 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 `@qml/PropertiesPanel.qml`:
- Around line 4411-4415: The findings Column currently remains visible during a
new scan; update its visible condition to also require that
AssetScanController.scanning is false so stale results are hidden during
in-flight scans—locate the Column block rendering findings (the Column with
width: parent.width - 16, spacing: 3) and change its visible expression from
AssetScanController.hasResults && AssetScanController.findings.length > 0 to
include !AssetScanController.scanning (matching the summary row gating).
- Around line 4449-4460: The onScanFinished handler currently updates
assetScanFeedback only when ok is true, dropping failure messages; update the
Connections handler for AssetScanController by adding an else branch (or
handling when ok is false) in function onScanFinished to set
assetScanFeedback.color to an error color (e.g. "`#c06060`") and
assetScanFeedback.text to message so failures surface to the UI; keep the
existing success branch for ok === true intact.
- Around line 4324-4387: The three MouseArea-only controls (folderBrowseMouse,
browserMouse, scanMouse) must support keyboard and assistive activation; update
each MouseArea to be focusable and expose Enter/Space handlers and accessibility
metadata: give the MouseArea a focus: true-able/tabbable target (wrap or set
focus on the parent Rectangle or add a FocusScope), add Keys.onPressed or
Keys.onReleased handlers for Qt.Key_Return and Qt.Key_Space that call the same
actions (AssetBrowserController.browseForDirectory(),
root.revealBottomTool("assetBrowser"), AssetScanController.scanFolder(...)), and
set accessible.role (Accessible.Button) and accessible.name (matching the Text)
so screen readers can announce them; ensure enabled respects
AssetScanController.scanning for scanMouse as before.
In `@src/AssetScanController_test.cpp`:
- Around line 55-65: The test reads persisted settings from a QSettings instance
created before calling AssetScanController::setSelectedProfileId(), which can
return stale data; after calling controller->setSelectedProfileId(id) create or
re-open a fresh QSettings (or call settings.sync()/settings.reload equivalent)
and then read AppSettingsKeys::validationPlatformProfileId() to assert
persistence. Locate the
AssetScanController::instance()/profileIds()/setSelectedProfileId()/selectedProfileId()
usage in the test and replace the final direct read from the original QSettings
with a newly constructed QSettings (or a synced one) before EXPECT_EQ to ensure
you assert the actual persisted value.
In `@src/AssetScanController.cpp`:
- Around line 302-303: The breadcrumb currently logs the full absolute path via
absRoot in SentryReporter::addBreadcrumb (in AssetScanController), which leaks
local directory paths; replace that usage with a non-sensitive identifier — e.g.
log only the profile id (m_selectedProfileId) and a sanitized path segment such
as QFileInfo(absRoot).fileName() or a hashed representation (use
QCryptographicHash::hash on absRoot) so breadcrumbs do not contain full absolute
paths; update the addBreadcrumb call to include the sanitized value instead of
absRoot and ensure any helper used for hashing/sanitizing is deterministic and
documented.
- Around line 253-254: Replace the blocking waitForFinished(-1) pattern with a
cancellable/timeout-aware approach: start the QProcess asynchronously (keep the
existing QProcess instance used where process.waitForFinished(-1) is called),
attach handlers for QProcess::finished and QProcess::errorOccurred to call
setScanning(false) and handle results, and add a QTimer watchdog (or use
waitForFinished(timeout) with a non-infinite timeout) that kills the process and
triggers setScanning(false) if the timeout expires; ensure the timeout/kill path
also emits the same finished/error handling so the controller/worker cannot
remain stuck in scanning state.
---
Nitpick comments:
In `@src/AssetScanController.cpp`:
- Around line 295-300: buildScanConfigWithPlatformProfile(m_selectedProfileId)
returns PlatformProfileScanSetup which includes setup.warnings but the current
code discards them; update the logic in the block that checks setup.ok to first
check if setup.warnings is non-empty and surface each warning (e.g., emit a
warning signal and/or log it to telemetry) before emitting error/scanFinished or
returning so partial profile-parse warnings are visible to users and telemetry;
reference PlatformProfileScanSetup, buildScanConfigWithPlatformProfile,
setup.warnings, m_selectedProfileId, and the existing emit error / emit
scanFinished calls when adding the warning handling.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: acc8a9c9-d72d-44dd-9a71-c9e416bf1da5
📒 Files selected for processing (12)
qml/BottomContextPanel.qmlqml/PropertiesPanel.qmlsrc/AppSettingsKeys.hsrc/AssetScanController.cppsrc/AssetScanController.hsrc/AssetScanController_test.cppsrc/CLIPipeline.cppsrc/CMakeLists.txtsrc/PlatformProfile.cppsrc/PlatformProfile.hsrc/mainwindow.cpptests/CMakeLists.txt
Co-authored-by: Cursor <cursoragent@cursor.com>
Cover JSON parsing, profile picker state, scan validation, and shared scan-config builder; add subprocess timeout, sanitize Sentry breadcrumbs, and QML keyboard/feedback fixes. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Co-authored-by: Cursor <cursoragent@cursor.com>



Summary
qtmesh scan --targetexecution that leaves the open scene untouched.AssetScanController(QML singleton) + sharedbuildScanConfigWithPlatformProfile()used by CLI and GUI.example-*profiles from the GUI picker; short display names; Asset Folder Scan collapsed by default.Test coverage
AssetScanController_test.cpp(11 tests): JSON parse success/failure, label sanitization, QSettings persistence/migration, profile list filtering, description, report ingestion, invalid scan path, CLI binary resolutionPlatformProfileScanSetupTest(3 tests): shared config builder matches loader, empty id, invalid idPlatformProfile_test.cppunchanged for bundled profile loadingReview fixes
scanningstatescanFinishedmessagesTest plan
./build_local/bin/UnitTests --gtest_filter="AssetScanController*:PlatformProfileScanSetup*"qtmesh scan <dir> --target modern-console --jsonCloses #370