feat: Update StateTransition for alerts

[parmesant]

Add previous_alert_state and previous_alert_duration. If not present at runtime, will get created using historical values

Fixes #XXXX.

Description

---

This PR has:

• been tested to ensure log ingestion and log query works.
• added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
• added documentation for new or modified features or behaviors.

Summary by CodeRabbit

• Refactor
  • Retain historical alert state transitions, recording the previous state and elapsed duration for each transition.
  • Expose alert list query parameters and their parser to enable external filtering, tagging and pagination controls.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

The PR adds prior-state tracking to StateTransition (previous alert state and its duration), updates its constructor and all creation sites to pass previous state/time, and makes ListQueryParams and parse_list_query_params public for external use. (29 words)

Changes

Cohort / File(s)	Change Summary
State Transition Tracking src/alerts/alert_structs.rs	Added previous_alert_state: Option<AlertState> and previous_state_duration: Option<i64> to StateTransition; changed StateTransition::new(...) to accept previous_alert_state and previous_alert_time and compute previous_state_duration and last_updated_at. Call sites updated to pass prior state/time when creating transitions.
HTTP Alerts Query Parameters src/handlers/http/alerts.rs	Made ListQueryParams and its fields (tags_list, offset, limit, other_fields_filters) public and changed fn parse_list_query_params(...) to pub fn parse_list_query_params(...).

Sequence Diagram(s)

(omitted — changes do not introduce a new multi-component control-flow requiring visualization)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Verify every StateTransition::new call site was updated and compiles.
• Confirm previous_state_duration is correctly computed from previous_alert_time and last_updated_at.
• Review alert update paths to ensure previous state/time are consistently supplied.

Possibly related PRs

• updated alerts module's visibility #1195 — similar visibility/export adjustments in the alerts area; related to exposing alerts types/functions.
• feat: save alert state history #1442 — prior changes to StateTransition/alert history types that this PR extends.

Suggested labels

for next release

Suggested reviewers

• nitisht
• nikhilsinhaparseable

Poem

🐰 I hopped through transitions, noted the state before,

minutes tucked behind me, stamped upon the door.
Queries now stand open, fields waved in light,
A rabbit's tiny cheer for changes done right! 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR description is largely incomplete. While it mentions the high-level goal (adding previous_alert_state and previous_alert_duration), it lacks the required sections: no detailed description of goals, no rationale for chosen solution, no key changes breakdown, and all verification checklist items remain unchecked.	Fill in the Description section with the goal, rationale, and key changes. Complete the checklist items confirming testing, comments, and documentation were added.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'feat: Update StateTransition for alerts' clearly summarizes the main change: updating the StateTransition struct for alert handling with new fields and modified methods.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2689945 and 7d9e9ef.

📒 Files selected for processing (1)

• src/alerts/alert_structs.rs (5 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-07-24T11:09:21.781Z

Learnt from: nikhilsinhaparseable
Repo: parseablehq/parseable PR: 1388
File: src/alerts/mod.rs:88-104
Timestamp: 2025-07-24T11:09:21.781Z
Learning: In the Parseable alert system (src/alerts/mod.rs), alert versions are server-generated and controlled via CURRENT_ALERTS_VERSION constant, not user input. The AlertVerison enum's From<&str> implementation correctly defaults unknown versions to V2 since the server only generates known versions (v1, v2). Unknown versions would only occur in exceptional cases like file corruption, making the current fallback approach appropriate.

Applied to files:

• src/alerts/alert_structs.rs

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)

• GitHub Check: Build Default x86_64-unknown-linux-gnu
• GitHub Check: Build Default aarch64-unknown-linux-gnu
• GitHub Check: Build Default x86_64-pc-windows-msvc
• GitHub Check: Build Default aarch64-apple-darwin
• GitHub Check: Build Kafka aarch64-apple-darwin
• GitHub Check: Build Kafka x86_64-unknown-linux-gnu
• GitHub Check: coverage
• GitHub Check: Quest Smoke and Load Tests for Distributed deployments
• GitHub Check: Quest Smoke and Load Tests for Standalone deployments

🔇 Additional comments (5)

src/alerts/alert_structs.rs (5)

760-763: LGTM! Well-structured addition for state transition tracking.

The new fields are clearly documented and appropriately typed as Option to handle initial transitions. This design enables backward compatibility with existing serialized data.

---

798-798: LGTM! Correct initialization of first transition.

Passing None for both previous_alert_state and previous_alert_time is appropriate for the initial state transition.

---

809-813: LGTM! Correct handling of state changes.

The method correctly passes the previous state and timestamp when creating a new transition, enabling proper duration tracking.

---

822-823: LGTM! Consistent handling of edge case.

Correctly handles the case when no previous states exist by passing None for both parameters, consistent with the initialization logic.

---

775-789: Breaking API change properly migrated.

All call sites for StateTransition::new have been updated to use the new signature with previous_alert_state and previous_alert_time parameters (lines 798, 809-810, 823). No unmigrated call sites found.

The concern about negative durations from clock skew is already addressed by the existing validation in get_recovery_times.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

src/handlers/http/alerts.rs (1)

47-52: Public struct exposed without doc comments on fields.

Making ListQueryParams public exposes it as part of the module's API. Consider adding doc comments to the public fields to help consumers understand the expected values and constraints (e.g., limit bounds, empty tags_list behavior).

 /// Query parameters for listing alerts
 pub struct ListQueryParams {
+    /// Comma-separated tag filters; empty if no tag filtering
     pub tags_list: Vec<String>,
+    /// Number of results to skip (default: 0)
     pub offset: usize,
+    /// Maximum results to return (1-1000, default: 100)
     pub limit: usize,
+    /// Additional field filters not covered by reserved params
     pub other_fields_filters: HashMap<String, String>,
 }

src/alerts/alert_structs.rs (1)

760-762: Add doc comment for previous_alert_state for consistency.

previous_state_duration has a doc comment but previous_alert_state does not. Consider adding one for consistency and clarity.

     pub last_updated_at: DateTime<Utc>,
+    /// The previous alert state before this transition, if any
     pub previous_alert_state: Option<AlertState>,
     /// Duration in seconds
     pub previous_state_duration: Option<i64>,

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 13b0568 and 62ae508.

📒 Files selected for processing (2)

• src/alerts/alert_structs.rs (5 hunks)
• src/handlers/http/alerts.rs (1 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-07-24T11:09:21.781Z

Learnt from: nikhilsinhaparseable
Repo: parseablehq/parseable PR: 1388
File: src/alerts/mod.rs:88-104
Timestamp: 2025-07-24T11:09:21.781Z
Learning: In the Parseable alert system (src/alerts/mod.rs), alert versions are server-generated and controlled via CURRENT_ALERTS_VERSION constant, not user input. The AlertVerison enum's From<&str> implementation correctly defaults unknown versions to V2 since the server only generates known versions (v1, v2). Unknown versions would only occur in exceptional cases like file corruption, making the current fallback approach appropriate.

Applied to files:

• src/alerts/alert_structs.rs

🔇 Additional comments (6)

src/handlers/http/alerts.rs (1)

55-55: LGTM!

Making parse_list_query_params public is consistent with exposing ListQueryParams. The function signature correctly returns a Result with proper error handling.

src/alerts/alert_structs.rs (5)

774-791: LGTM! Clean implementation of previous state tracking.

The constructor correctly:

• Captures current time once and reuses it for both last_updated_at and duration calculation.
• Calculates duration only when previous_alert_time is provided.
• Uses Option types appropriately for backward compatibility with existing data.

---

800-800: LGTM!

Correctly initializes with None for both optional fields when creating the initial state entry.

---

811-815: LGTM! Proper state transition tracking.

When the state changes, the code correctly captures:

• The previous state from last_transition.state
• The previous timestamp from last_transition.last_updated_at

This enables accurate duration calculations and state history tracking.

---

824-825: LGTM!

The fallback case for an empty states list correctly passes None for both optional parameters, which is the appropriate behavior when there's no prior state to reference.

---

754-763: Backward compatibility is guaranteed by Serde's standard behavior.

New Option fields will automatically deserialize as None for existing serialized data that lacks these fields. Serde 1.0 provides this behavior without requiring #[serde(default)] attributes. The existing deserialization code in object_store_metastore.rs (lines 231, 245, 278) already handles this correctly with graceful error handling, so no migration logic or storage changes are needed.