ALEC-301: Fix LLM clustering engine correctness (follow-up to #157)

[joseanesONMS]

Follow-up to #157 — post-merge review of the original ALEC-301 LLM-Based Correlation surfaced six defects; this corrects them. The engine could silently stay inactive under a valid config, bypass the token budget, and lacked the topology it needs.

P1 — correctness

1. Clustering no longer gated on the RCA enabled flag. readLlmConfig required enabled — the Root Cause Analysis toggle — so an operator with endpoint/model/key set but RCA off got null every tick. Clustering now requires only the shared connection fields; selecting the engine is the signal to use it.
2. Shared token budget enforced + recorded. Each tick checks UTC daily/monthly usage against the limits (same semantics as the RCA TokenBudget) and writes each call's usage into the shared ALEC_LLM_USAGE store — so clustering both respects the cap and shows up in the usage dashboard.
3. Topology is sent to the model. The request now serializes device adjacency (from the cluster graph) and tags each alarm with its device, so the model can group by connectivity as the prompt/UI/demo assume.

P2 — robustness

4. Request bounded at MAX_ALARMS (200) — cluster the most-recent alarms rather than overflow the context window.
5. Membership dedup — each alarm to at most one cluster (global + within-group), drop sub-2-alarm groups.
6. Deleted edges reconciled — the periodic topology refresh removes edges ALEC still holds that EdgeDao no longer returns (missed delete callback); logic factored into a shared removeEdgeInventory().

Verification

• 34 engine/llm + 20 datasource tests green, with new coverage for every fix.
• Budget/usage records are written directly to the shared KV store in the same shape as the RCA UsageStore (the engine bundle can't depend on features/llm-suggestions).

🤖 Generated with Claude Code

https://claude.ai/code/session_01UB6PGc2rpPbojnqbTHU5ND

[joseanesONMS]

@cgorantla — ready for review. This is the follow-up to #157 that corrects the six defects your post-merge review found in the ALEC-301 LLM clustering engine (there is no separate ticket — this is part of the original ALEC-301 work).

P1 (correctness):

1. Clustering no longer gates on the RCA enabled flag — it required only the shared endpoint/model/key, so the engine no longer sits silently inactive under a valid config.
2. The shared daily/monthly token budget is now enforced and recorded (same UTC-window semantics + record shape as the RCA TokenBudget/UsageStore, written directly to ALEC_LLM_USAGE since the engine bundle can't depend on features/llm-suggestions).
3. Device topology (adjacency between alarm-bearing vertices) is serialized into the request, so the model can group by connectivity as the prompt/UI/demo assume.

P2 (robustness): request bounded at 200 alarms; alarm memberships deduped (global + within-group) with the ≥2-per-group rule enforced; and the periodic edge-topology refresh now reconciles missed deletions instead of leaving stale inventory.

All CircleCI stages green; 34 engine/llm + 20 datasource tests pass, with new coverage for every fix. I also built this and deployed it to a local OpenNMS — ALEC boots and the fixed engine.llm bundle loads (verified the budget/topology/usage changes are in the running class).

[joseanesONMS]

@cgorantla — addressed the four round-2 findings (commit cb5450a0):

1. Edge-reconciliation race — refreshEdgeTopology now snapshots the held edge IDs before the poll and only removes IDs from that snapshot the poll no longer returns. An edge added concurrently during the poll isn't in the snapshot, so it can never be deleted as "stale."
2. Budget scan under the graph lock — the check is hoisted into tick() (outside the graph-locked cluster()), and it now reads a cached day/month total refreshed by a full scan only on a UTC period rollover or every 5 min; clustering's own spend is folded into the cache immediately so enforcement stays accurate between rescans. (The 30 s blocking HTTP call is still inside the lock — that's the pre-existing ALEC-301: LLM-based clustering engine (phases 3a–3b) + topology refresh #157 issue you noted this PR doesn't tackle; happy to do it here or as a follow-up.)
3. configureAndStoreLlm null/≤0 frequency — clamps null/≤0 to LlmEngineFactory.DEFAULT_CLUSTER_FREQUENCY_MS. I clamped rather than 400'd on purpose: the constructor replays persisted config through this path and swallows the Response, so a bad record must still yield a usable engine.
4. UI default prompt — AccountSettings.vue's DEFAULT_CLUSTER_PROMPT is now the engine's exact prompt (omit singletons, ≥2 per group), so it no longer tells the model to emit singletons the parser discards. Noted the drift-proof follow-up (serve it from the server like the RCA defaultSystemPrompt).

New tests: budget-cache behavior, and null/zero clusterFrequency clamping. All engine/llm + features/ui + datasource + UI suites green; ui-ext bundle rebuilt. CI re-running.

[joseanesONMS]

Pushed 3746abc6 implementing the two optional hardening items from the review:

1. HTTP call hoisted out of the correlation graph lock. cluster() previously blocked the graph lock for the full ~30s OpenAI round-trip. It now returns the latest resolved groups immediately and dispatches the request on a dedicated daemon executor (llm-cluster-http), with a single in-flight request guarded by an AtomicBoolean so ticks can't pile up. The parse is split:

• parseGroups(json, om) → raw id groups, runs on the HTTP thread, off-lock.
• resolveClusters(idGroups, alarmsById) → maps ids to clusters against the live alarm map (applies the ≥2 rule), runs under the lock.

Budget totals are now cached behind budgetLock with a 5-minute rescan, so the budget check no longer scans the usage store on every tick.

2. Default cluster prompt served from the server. LlmClusterEngine.DEFAULT_CLUSTER_PROMPT is now public and exposed via LlmConfigStatus.getDefaultClusterPrompt(). AccountSettings.vue hydrates "Reset to default" from that value instead of a hard-coded copy, so the UI text can't drift from what the engine enforces. TUser gains the defaultClusterPrompt field.

New tests: parseGroupsReturnsRawIdGroupsWithoutAlarmResolution + resolveClustersMapsIdGroupsAgainstCurrentAlarms (engine), statusServesEngineDefaultClusterPrompt (rest). Full suite green (engine/llm 37, features/ui 54, ui 86); UI bundle rebuilt.

[joseanesONMS]

@cgorantla this is ready for another look. Both optional hardening items from your review are now implemented and pushed (3746abc6):

1. HTTP call hoisted out of the correlation graph lock — cluster() returns the latest resolved groups immediately and dispatches the ~30s OpenAI request on a dedicated daemon executor (single in-flight, guarded by an AtomicBoolean). Parse split into off-lock parseGroups() and under-lock resolveClusters(); budget totals cached with a 5-min rescan.
2. Default cluster prompt served from the server — LlmClusterEngine.DEFAULT_CLUSTER_PROMPT is now public and exposed via LlmConfigStatus.getDefaultClusterPrompt(); the UI hydrates "Reset to default" from it instead of a hard-coded copy.

All CI green (pre-build, build, build-debian, build-docs, smoke-test-commit). Full test suite passes (engine/llm 37, features/ui 54, ui 86) and I've verified both changes on a live OpenNMS deploy. Details in the comment above.

[joseanesONMS]

Good catches — both fixed in f7e548bc.

Concern 1 (results lagging a full tick period). Root cause: the configured cluster frequency was wired straight to tickResolutionMs, so it governed both how often we query the LLM and how often onTick applies the grouping / runs GC / processes feedback. At the 1-hour default that meant a grouping returned at 10:00 didn't become situations until 11:00.

Rather than call tick() from the HTTP callback (unsafe — AbstractClusterEngine's tick-state assumes the single driver tick thread, exactly as you noted), I decoupled the two cadences:

• The engine now ticks at a fast reconcile interval (RECONCILE_INTERVAL_MS = 30s, capped at the frequency for sub-30s configs). Every reconcile tick re-resolves latestGroups against the current alarms and runs GC/feedback — so a freshly-returned grouping becomes situations within ~one reconcile tick.
• A new LLM request is throttled separately to the configured frequency via clusterRequestIntervalMs + lastRequestAtMs (gate in maybeStartClusteringRequest).

So the expensive query still happens hourly, but effective correlation latency drops from 1–2× frequency to ~one reconcile tick. Everything stays on the driver's single tick thread; the HTTP thread only writes the volatile latestGroups. This felt cleaner than a driver re-tick hook and doesn't touch the driver API — happy to revisit if you'd prefer the documented-lag route instead.

Concern 2 (budget-blocked tick skips super.tick()). Fixed exactly as you suggested: tick() always calls super.tick() now; the budget gates only the outbound request. GC, feedback and re-resolution keep running even when the cap is hit. The "budget reached" WARN moved to the false→true transition so it isn't logged every reconcile tick.

Minor: readLlmConfig() now runs once per tick — the config is read on the tick thread in tick() (tickConfig) and reused by cluster(). (The 5-min budget rescan is unchanged and still off the graph lock.)

Tests updated: the factory now asserts the frequency drives the request interval (not the reconcile tick) and that the reconcile tick never exceeds the frequency. Full suite green (engine/llm 38, features/ui 54).

[cgorantla]

LGTM!

[cgorantla]

This needs to run -smoke in branch name. That will run all the smoke tests.