ALEC-296: Redesign correlation engine configuration UI#154
Open
joseanesONMS wants to merge 4 commits intoOpenNMS-Plugins:developfrom
Open
ALEC-296: Redesign correlation engine configuration UI#154joseanesONMS wants to merge 4 commits intoOpenNMS-Plugins:developfrom
joseanesONMS wants to merge 4 commits intoOpenNMS-Plugins:developfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 16 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
joseanesONMS
added a commit
to joseanesONMS/alec
that referenced
this pull request
May 5, 2026
Addresses Copilot review feedback on PR OpenNMS-Plugins#154. Without this, reEvaluateAllOpenAlarms() (and the existing engine-config save paths) could report HTTP 200 even when the driver init was interrupted before the engine came back up — masking failures from the caller. (The companion test fix for the missing re-evaluate confirm-cancel no-op test was already pushed by Copilot Autofix as 19bc094.)
3 tasks
Rebased onto current develop after PR OpenNMS-Plugins#152 (ALEC-298) removed the cloud-feedback gRPC and data-sharing agreement flow. The agreement section that lived in the original commit on this branch is gone — it depended on infrastructure that no longer exists. The rest of the ALEC-296 scope is preserved: UI (ui/src/containers/AccountSettings.vue): - Page header renamed: "Configuration Page" → "Correlation Engine Configuration Page". - Deep Learning option hidden from the engine selector (the bundle is intentionally NOT unregistered: it remains available for tests and regression scenarios; this is a UI-only filter). - New disabled "LLM Based" radio with a "Coming soon" caption as a placeholder for the upcoming engine option. - DBScan correlation variables (alpha, beta, epsilon) editable as numeric inputs visible only when Clustering is selected; wired through useUserStore.setEngineInfo() (now accepts an overrides arg) into the existing /alec/engine/configuration endpoint, which already accepted these values — previously the UI sent only engineName and distanceMeasureName. - Click-toggle help popover next to the variables title with bulleted guidance + default values for each variable. (Feather's tooltip is string-only, hence a popover for bullets / inline code / italics.) - Circular-arrow Restore button to reset variables to defaults in one click. - Two action buttons at the bottom (left of Save Changes), each prompting window.confirm before firing: * "Close All Open Situations" → POST /alec/situation/closeAll * "Re-Evaluate All Open Alarms" → POST /alec/engine/reEvaluate Backend (features/ui): - SituationRest + SituationRestImpl: new POST /alec/situation/closeAll endpoint. Walks every situation that is not already ACCEPTED/REJECTED and forwards an empty-alarm REJECTED situation to OpenNMS — same mechanism the existing per-situation reject path uses. Post-OpenNMS-Plugins#152 the cloud-client (gRPC) sendSituation step the original commit had is intentionally absent. - EngineRest + EngineRestImpl: new POST /alec/engine/reEvaluate endpoint. Calls the existing driverInit() (destroy + initAsync) so the engine re-applies the current correlation variables to all currently active alarms. - driverInit() now returns an error Response on InterruptedException (was: silently returned null and let the caller report 200). Addresses Copilot review feedback from the original PR OpenNMS-Plugins#154. Tests: - UI: 13 vitest specs in src/tests/accountSettings.test.ts covering the new title, Deep-Learning removal, disabled LLM-Based + caption, variables form, save round-trip with alpha/beta/epsilon, help popover toggle (bullets + defaults), reset to defaults, the Clustering-only visibility rule, and both action buttons confirming and dispatching plus their no-op-on-cancel paths. - Backend: extend EngineRestImplTest with a reEvaluate-triggers-init test and an alpha/beta/epsilon-applied-to-factory test; extend SituationRestImplTest with closeAllOpenSituations marking every non-closed situation as REJECTED.
joseanesONMS
force-pushed
the
ja/jira/ALEC-296
branch
from
1b43755 to
f4fef98
Compare
…ization Annotated the EngineParameterImpl Builder with @JsonIgnoreProperties(ignoreUnknown = true) so persisted engine config can be read back even when it contains fields added by a future version (e.g. ALEC-297's noPathDistance). Without this, dropping back to a build that doesn't know about the newer field caused the features.ui bundle to fail to start with: Unrecognized field "noPathDistance" (class EngineParameterImpl$Builder), not marked as ignorable Reproduced locally by deploying this branch on top of a KV store that had a noPathDistance value persisted by an earlier ALEC-297 build. Added EngineParameterImplTest.deserializeIgnoresUnknownFields to lock in the new behaviour.
Previously the system default was alarminspaceandtimedistance: the DBScan blueprint's cm:property and DBScanEngine.DEFAULT_DISTANCE_MEASURE both pointed at it, and the UI checkbox was only ticked when the persisted config explicitly said "hellinger". On a brand-new install that meant Clustering ran with the spatial-time measure even though Hellinger is the recommended default. Switched the default to Hellinger in both layers: - engine/dbscan blueprint: cm:property name="distanceMeasure" now defaults to "hellinger" (matches the lookup-map key, which the alarmInSpaceAndTimeDistanceMeasureFactory bean-id form did not — the old value was triggering the "Wrong distance measure configuration ..." fallback warning at startup). - DBScanEngine.DEFAULT_DISTANCE_MEASURE = "hellinger". This also flows into EngineParameterImpl.getDistanceMeasureName()'s null fallback, so REST clients that don't supply a distanceMeasureName read back "hellinger". - AccountSettings.vue: when there is no persisted engineInfo (fresh install), default the Hellinger checkbox to checked. When there is a saved config, honour what's persisted as before. Tests: extended accountSettings.test.ts with a fresh-system check asserting hellinger.value === true when engineInfo is null.
deserializeDefaultStringBuilder asserted that getEpsilon() returned null when only engineName was supplied. That happened to be true before, because DBScanEngine.DEFAULT_DISTANCE_MEASURE was "alarminspaceandtimedistance", and getEpsilon()'s switch matches "alarminspacetime" (not "alarminspaceandtimedistance"), so the no-match branch returned null. After the previous commit set the DBScan default to "hellinger", getEpsilon()'s switch now picks up the "hellinger" case and returns HellingerDistanceMeasure.DEFAULT_EPSILON (75.0). The test expectation was wrong: it was relying on a switch-case fallthrough rather than an intentional null. Updated to assert HellingerDistanceMeasure.DEFAULT_EPSILON. Local test runs masked this because Java compile-time-constant inlining kept the old DEFAULT_DISTANCE_MEASURE value baked into EngineParameterImpl.class until a clean rebuild — CI does a clean build, which is how this surfaced (CircleCI build 8631 on PR OpenNMS-Plugins#154).
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
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
Reworks the Correlation Engine Configuration page (
AccountSettings.vue) and the onboarding flow (ConfigurationPage.vue):EngineParameter— previously the UI only sentengineName+distanceMeasureName.confirm()before firing:POST /alec/situation/closeAllPOST /alec/engine/reEvaluateBackend additions
SituationRest: newPOST /alec/situation/closeAll. Walks every situation that is not alreadyACCEPTED/REJECTED, marks itREJECTED, sends to the cloud client, and forwards an empty-alarmREJECTEDsituation to OpenNMS — same mechanism the existing per-situation reject path uses.EngineRest: newPOST /alec/engine/reEvaluate. Calls the existingdriverInit()(destroy +initAsync) so the engine re-applies the current correlation variables to all currently active alarms.Out of scope
CrossDeviceCorrelationIT(marked@Ignore'd) and tracked separately as ALEC-297. With no topology path between two devices,AlarmInSpaceTimeDistanceMeasure.compute()returns ~10⁹–10¹⁰, which no realistic ε can swallow. Empirical sweep in the test confirms epsilon ≥ 1e10 is required to cluster cross-device alarms — clearly unusable in practice. Fix (configurable no-path penalty, end-to-end plumbing) is intentionally deferred.Ticket: ALEC-296
Test plan
mvn -pl features/ui test(extendsEngineRestImplTestwithtestSetDbScanAppliesAlphaBetaEpsilonToFactory+testReEvaluateAllOpenAlarmsTriggersDriverReinit; extendsSituationRestImplTestwithcloseAllOpenSituations_marksAllNonClosedAsRejected).npm testinui/(12 tests inaccountSettings.test.tscovering title, Deep-Learning removal, LLM-Based disabled + caption, variables form, save round-trip, help popover toggle + bullets + defaults, reset to defaults, engine-switch hide, both action buttons confirming and dispatching, and the no-op path when confirm is dismissed).npm run buildinui/andmvn clean install -DskipTestsproduce a fresh KAR.POST /opennms/rest/alec/situation/closeAll→ 200,POST /opennms/rest/alec/engine/reEvaluate→ 200.