fix(alert): keep evaluation state on cosmetic rule edits and toggles

[89799969]

Fixes #4326

Problem

Every successful updateRule and toggleRule call unconditionally runs alertStateRepository.deleteByRuleId(id). That wipes the live FIRING / ACK / PENDING episode even when the evaluation identity did not change.

Consequences:

• Renaming a rule (or editing only description/channels) loses the active episode; the next evaluation can re-fire and re-notify.
• Disabling then re-enabling a rule never keeps ACK status, so a temporary mute-and-restore looks like a brand-new alert.

AlertRuleSemanticFingerprint already encodes the fields that decide firing (metric, threshold, operator, duration, scope, samples, …) and is used for duplicate detection.

Fix

• updateRule loads the existing rule and deletes state only when the semantic fingerprint changes.
• toggleRule / bulkToggleRules no longer delete state (enable/disable is not an evaluation change).
• Rule delete still clears state.

Testing

• AlertServiceTest (79 tests), including:
  • updatingRuleNameWithoutChangingEvaluationConditionsKeepsStateTest
  • updatingRuleThresholdResetsEvaluationStateTest
  • togglingRuleKeepsEvaluationStateTest
  • updatingRuleShouldResetItsPreviousEvaluationStateTest (existing, still passes when no prior rule row)

Tests run: 79, Failures: 0, Errors: 0, Skipped: 0

Local run: Temurin 21.0.12.1, mvn -Dtest=AlertServiceTest test

[RockteMQ-AI]

Summary\nThis PR fix(alert): keep evaluation state on cosmetic rule edits and toggles modifies 2 files (+58 -8).\n\n### Findings\n- [Info] Long string literal: consider extracting to a constant\n\nPlease review the findings above.\n\n---\nAutomated review by "github-manager-bot"

[lizhimins]

Half of this is a real fix and half introduces a new risk; please split it.

The real fix: AlertService.updateRule (:207) calls deleteByRuleId unconditionally, so renaming a rule or editing its description clears FIRING/ACK state and AlertStateMachine.advanceHit (:71-87) re-fires from scratch. Making that cleanup conditional on the fingerprint actually changing is right, and the three new tests plus the six revised deleteByRuleId assertions check out.

The new risk: the patch also drops the cleanup on toggle, and nothing converges the state of a disabled rule — MybatisPlusAlertStateRepository.java:105 and NativeAlertProcessor.java:104 both filter on enabled. After a rule is disabled and re-enabled, a stale FIRING row stops advanceHit from emitting a new FIRING event, which can swallow a genuinely new incident.

Please keep the fingerprint condition and either retain the disable-time cleanup or add convergence for disabled rules.

[89799969]

Split per review:

• Kept the real fix: fingerprint-gated cleanup on updateRule so cosmetic edits no longer drop FIRING/ACK.
• Restored disable-time cleanup (clearStateWhenDisabling) for single and bulk toggle, so a disabled rule cannot leave a stale FIRING row that swallows a new incident after re-enable.

AlertServiceTest updated and green locally.

[89799969]

@lizhimins Following up - fingerprint-gated update cleanup kept; disable-time state cleanup restored for single and bulk toggle (a42d2741). Branch is MERGEABLE/CLEAN on current rocketmq-studio. Ready for re-review.

[RockteMQ-AI]

Summary

Reviewed PR #4327: fix(alert): keep evaluation state on cosmetic rule edits and toggles

The changes look reasonable. No critical issues identified.

Observations

• 2 files changed
• Test coverage: test files included ✓

---

Automated review by github-manager-bot