use crate::alerts::{self, AlertRuntimeState};
use crate::data_value::{DataValue, MetricPrimitiveKind, ParamDeclaration};
use crate::dto::{
    BucketValueDto, LogEntryDto, LogsParams, MetricDeclarationDto, ParamDeclarationDto,
    PromqlSeriesDto, StatsDto, TimelineBucketDto, TimelineParams,
};
use crate::filter_datetime;
use crate::filter_parser::{self, CompareOp, FilterExpr, FilterValue};
use crate::index_key::IndexKey;
use crate::metric_primitive::{MetricPrimitive, Number};
use crate::promql::{self, AggOp, LabelMatcher, PromqlQuery};
use crate::rate;
use crate::settings::SettingsHandle;
use crate::timestamp::Timestamp;
use regex::Regex;
use serde::Deserialize;
use std::cmp::Ordering;
use std::collections::{BTreeMap, HashMap, HashSet};
use tokio::sync::{mpsc, oneshot};

pub struct IngestedLine {
    pub sequence: u64,
    pub timestamp: Timestamp,
    pub text: String,
}

/// One sample scraped from a Prometheus/OpenMetrics endpoint, pushed straight into a metric's bucket
/// without going through log-line regex matching — see `crate::ingest::run_metrics_poll`.
pub struct MetricSample {
    pub metric_name: String,
    pub labels: BTreeMap<String, String>,
    pub value: f64,
    pub timestamp: Timestamp,
}

/// Also the request body for `POST /api/params` — `Deserialize` is derived directly here rather than adding a separate DTO, since the shape already matches `ParamDeclaration`.
pub type DeclareParamRequest = ParamDeclaration;

/// Response to `EngineMessage::ClearParams` (ticket 052) — also the JSON body for `DELETE /api/params`.
#[derive(Debug, Clone, serde::Serialize)]
pub struct ClearParamsResult {
    /// How many param declarations (manual + auto-derived) were removed.
    pub removed: usize,
    /// How many params `auto_declare_params` re-derived immediately afterward from retained log text.
    pub reextracted: usize,
    /// How many `IndexKey::IngestedLog` entries were still retained (not yet evicted by `max_logs`)
    /// and so were re-scanned — surfaced so a caller can tell the user re-extraction only covers
    /// logs still held in memory, not lines evicted before the clear.
    pub logs_considered: usize,
}

/// A metric references its params by name (resolved against the engine's standalone
/// `ParamDeclaration` store in `apply_metric_to_log`) rather than embedding its own
/// regex/capture-group/kind per param — see ticket 037. Also the request body for
/// `POST /api/metrics` (hence `Deserialize`).
#[derive(Debug, Clone, Deserialize)]
pub struct DeclareMetricRequest {
    pub name: String,
    pub message_match_regex: String,
    /// Names of already-declared params whose values become this metric's bucket labels; `#[serde(default)]` so a label-less metric (e.g. matched/unmatched-labels-only) doesn't need an empty array.
    #[serde(default)]
    pub label_params: Vec<String>,
    /// Name of an already-declared param (if any) whose value becomes this metric's per-event bucket value instead of a label.
    #[serde(default)]
    pub value_param: Option<String>,
    /// Fixed labels layered on top of what `label_params` captured, so e.g. a case-insensitive match doesn't fragment into "ERROR"/"Error" series; `#[serde(default)]` so existing declarations are unaffected.
    #[serde(default)]
    pub matched_labels: Option<BTreeMap<String, String>>,
    /// When a line doesn't match, registers one occurrence labeled with these fixed values instead of being invisible to this metric — plain regex alternation can't express "didn't match" without lookaround; `#[serde(default)]` so existing declarations keep matching-only behavior.
    #[serde(default)]
    pub unmatched_labels: Option<BTreeMap<String, String>>,
}

#[allow(dead_code)]
pub enum EngineQuery {
    Logs(LogsParams),
    Timeline(TimelineParams),
    Stats,
    ParamDeclarations,
    MetricDeclarations,
    /// `labels_filter` compares against each label's `Display` form (not a typed value), since the filter arrives as plain strings off the query string.
    BucketValues {
        metric_name: String,
        labels_filter: Option<BTreeMap<String, String>>,
        start_ms: Option<i64>,
        end_ms: Option<i64>,
        since_sequence: Option<u64>,
        limit: Option<usize>,
    },
    /// `query` is parsed inside the engine (not the API handler) so it stays in the single-owner loop with no copying; a malformed query is only discovered here, hence `EngineResponse::Promql` carries a `Result`.
    Promql { query: String, start_ms: Option<i64>, end_ms: Option<i64> },
}

#[allow(dead_code)]
pub enum EngineMessage {
    Ingest(IngestedLine),
    IngestMetricSample(MetricSample),
    DeclareParam {
        request: DeclareParamRequest,
        respond_to: oneshot::Sender<Result<(), String>>,
    },
    RemoveParamDeclaration {
        name: String,
        respond_to: oneshot::Sender<bool>,
    },
    /// Ticket 052: wipes the entire standalone-param set (manual *and* auto-derived — unlike
    /// `reload_engine_declarations`'s profile-switch cleanup, this is an explicit user-driven reset,
    /// not a "keep engine-discovered facts across profiles" cleanup), then immediately re-runs
    /// `auto_declare_params` over every `IndexKey::IngestedLog` entry still retained in the index,
    /// oldest to newest, so declaration-order semantics match a fresh ingest. Lines already evicted
    /// by `max_logs` can't regain their params — `ClearParamsResult::logs_considered` reports how
    /// many retained lines were re-scanned, so a caller can surface that bound to the user.
    ClearParams {
        respond_to: oneshot::Sender<ClearParamsResult>,
    },
    DeclareMetric {
        request: DeclareMetricRequest,
        respond_to: oneshot::Sender<Result<(), String>>,
    },
    /// Updates an existing metric declaration named `name` in place — same validation as `DeclareMetric`, but fails if no declaration named `name` exists (rather than silently creating one), and re-backfills bucket data from scratch so edited params/regex don't leave stale buckets from the old declaration mixed in.
    UpdateMetric {
        name: String,
        request: DeclareMetricRequest,
        respond_to: oneshot::Sender<Result<(), String>>,
    },
    RemoveMetricDeclaration {
        name: String,
        respond_to: oneshot::Sender<bool>,
    },
    /// `None` means unlimited; sent on startup and whenever the setting changes so the cap applies without a restart.
    SetMaxLogs(Option<usize>),
    Query {
        query: EngineQuery,
        respond_to: oneshot::Sender<EngineResponse>,
    },
}

#[allow(dead_code)]
pub enum EngineResponse {
    Logs(Vec<LogEntryDto>),
    Timeline(Vec<TimelineBucketDto>),
    Stats(StatsDto),
    ParamDeclarations(Vec<ParamDeclarationDto>),
    MetricDeclarations(Vec<MetricDeclarationDto>),
    BucketValues(Vec<BucketValueDto>),
    /// `Err` holds a human-readable message for a malformed query, which the API layer maps to a 400 response.
    Promql(Result<Vec<PromqlSeriesDto>, String>),
    /// Kept as a variant (rather than making `handle_query` return a `Result`) so every query path still funnels through the same `oneshot::Sender<EngineResponse>`.
    Error(String),
}

pub type EngineHandle = mpsc::Sender<EngineMessage>;

/// Sole owner of the primary index; runs single-threaded so the map never needs a lock, and builds DTOs itself so callers never clone the index.
pub async fn run(mut rx: mpsc::Receiver<EngineMessage>, settings: SettingsHandle) {
    let mut index: BTreeMap<IndexKey, DataValue> = BTreeMap::new();
    let mut max_logs: Option<usize> = None;
    let mut log_count: usize = 0;
    // Its own counter, independent of log `sequence` — scraped samples aren't tied to any ingested line.
    let mut metric_sample_sequence: u64 = 0;
    // Per-rule rolling windows/seen-value sets, keyed by rule id; lives only as long as this loop (no restart persistence).
    let mut alert_states: HashMap<String, AlertRuntimeState> = HashMap::new();
    // Ticket 011: running total of `approx_log_bytes` (see that function) across
    // every currently-ingested log, kept in lockstep with `log_count` — incremented
    // here on ingest, decremented in `evict_excess_logs` when a log is dropped.
    let mut approx_bytes_total: u64 = 0;
    // Ticket 060/061: one entry per param name that's ever been used as a `sort_by`, populated
    // lazily on first sorted query and kept incrementally up to date on every ingest thereafter;
    // swept for staleness on ingest too (see `purge_stale_sort_helper_indexes`).
    let mut sort_helper_indexes: HashMap<String, SortHelperIndexEntry> = HashMap::new();
    // Ticket 059: the most recently seen `log stream` header line's column names, cached across
    // ingested lines since a `log stream` pipe only prints its header once, before any data line.
    let mut log_stream_header: Option<crate::logfmt::LogStreamHeader> = None;

    while let Some(message) = rx.recv().await {
        match message {
            EngineMessage::Ingest(line) => {
                let IngestedLine { sequence, timestamp, text } = line;
                approx_bytes_total += approx_log_bytes(&text);
                index.insert(
                    IndexKey::IngestedLog { sequence, timestamp },
                    DataValue::LogLiteral { text: text.clone() },
                );
                log_count += 1;
                auto_declare_log_stream_params(&mut index, &mut log_stream_header, &text);
                auto_declare_params(&mut index, &text);
                apply_all_metrics(&mut index, sequence, timestamp, &text);
                update_sort_helper_indexes_on_ingest(&mut sort_helper_indexes, &index, sequence, timestamp, &text);
                purge_stale_sort_helper_indexes(&mut sort_helper_indexes);
                evict_excess_logs(&mut index, &mut log_count, max_logs, &mut approx_bytes_total);

                // Evaluate alert rules against just this line, then hand any fired rule off to delivery via tokio::spawn so it stays off the hot path.
                let (rules, ntfy_topic, ntfy_server) = {
                    let settings = settings.lock().unwrap();
                    (settings.alert_rules.clone(), settings.ntfy_topic.clone(), settings.ntfy_server.clone())
                };
                let fired =
                    alerts::evaluate_alerts(&rules, &mut alert_states, &index, sequence, timestamp, &text);
                for (rule, message) in fired {
                    tokio::spawn(alerts::deliver_for_rule(rule.name, message, ntfy_topic.clone(), ntfy_server.clone()));
                }
            }
            EngineMessage::IngestMetricSample(sample) => {
                let labels = sample
                    .labels
                    .into_iter()
                    .map(|(name, value)| (name, MetricPrimitive::String(value)))
                    .collect();
                push_bucket_value(
                    &mut index,
                    &sample.metric_name,
                    labels,
                    metric_sample_sequence,
                    sample.timestamp,
                    MetricPrimitive::Number(Number::Float(sample.value)),
                );
                metric_sample_sequence += 1;
            }
            EngineMessage::SetMaxLogs(new_max) => {
                max_logs = new_max;
                evict_excess_logs(&mut index, &mut log_count, max_logs, &mut approx_bytes_total);
            }
            EngineMessage::DeclareParam { request, respond_to } => {
                // A manual re-declaration can change the regex/kind under the same name — drop any
                // helper index built under the old declaration so a sorted query rebuilds it fresh
                // rather than serving values derived from the now-stale regex.
                sort_helper_indexes.remove(&request.name);
                let result = declare_param(&mut index, request);
                let _ = respond_to.send(result);
            }
            EngineMessage::RemoveParamDeclaration { name, respond_to } => {
                let existed = index.remove(&IndexKey::ParamDeclaration { name: name.clone() }).is_some();
                // A helper index built under the old declaration (or a same-named one re-declared
                // with a different regex/kind later) would otherwise keep serving stale derived
                // values until ticket 061's sweep happens to purge it — dropping it here instead
                // means the next sorted query for this param rebuilds it fresh.
                sort_helper_indexes.remove(&name);
                let _ = respond_to.send(existed);
            }
            EngineMessage::ClearParams { respond_to } => {
                let removed = clear_all_param_declarations(&mut index);
                let (reextracted, logs_considered) = reextract_params_from_retained_logs(&mut index);
                sort_helper_indexes.clear();
                let _ = respond_to.send(ClearParamsResult { removed, reextracted, logs_considered });
            }
            EngineMessage::DeclareMetric { request, respond_to } => {
                let result = declare_metric(&mut index, request);
                let _ = respond_to.send(result);
            }
            EngineMessage::UpdateMetric { name, request, respond_to } => {
                let result = update_metric(&mut index, &name, request);
                let _ = respond_to.send(result);
            }
            EngineMessage::RemoveMetricDeclaration { name, respond_to } => {
                let existed = remove_metric_declaration(&mut index, &name);
                let _ = respond_to.send(existed);
            }
            EngineMessage::Query { query, respond_to } => {
                let response = handle_query(&index, query, approx_bytes_total, &mut sort_helper_indexes);
                let _ = respond_to.send(response);
            }
        }
    }
}

/// Validates the regex and capture group before storing, so a bad declaration is rejected up front rather than discovered later.
fn declare_param(
    index: &mut BTreeMap<IndexKey, DataValue>,
    request: DeclareParamRequest,
) -> Result<(), String> {
    if OTTYL_RESERVED_PARAM_NAMES.contains(&request.name.as_str()) {
        return Err(format!(
            "'{}' is a reserved name, present on every log line without declaring anything; \
             it can't be redeclared as a param",
            request.name
        ));
    }
    if let Some(DataValue::ParamDeclaration(existing)) =
        index.get(&IndexKey::ParamDeclaration { name: request.name.clone() })
    {
        if existing.auto_derived {
            return Err(format!(
                "param '{}' was auto-detected from the log stream and can't be edited",
                request.name
            ));
        }
    }
    // An `elapsed_since_last` param is never text-extracted at all, so `message_match_regex` is
    // irrelevant to it — but the field always compiles as a regex regardless of extractor (an empty
    // pattern is valid and matches trivially), so it's harmless to still require it to parse.
    let re = Regex::new(&request.message_match_regex)
        .map_err(|err| format!("invalid regex: {err}"))?;
    if request.elapsed_since_last && request.kind != MetricPrimitiveKind::Duration {
        return Err("elapsed_since_last params must declare kind 'duration'".to_string());
    }
    // A Boolean-kind param never uses a capture group — its value is just whether the regex matched.
    // Neither does an `elapsed_since_last` param — its value is computed, not captured.
    let uses_json_field = request.json_field_path.as_deref().is_some_and(|path| !path.is_empty());
    if request.kind != MetricPrimitiveKind::Boolean
        && !request.elapsed_since_last
        && !uses_json_field
        && !re.capture_names().flatten().any(|name| name == request.capture_group)
    {
        return Err(format!(
            "regex has no capture group named '{}'",
            request.capture_group
        ));
    }

    index.insert(
        IndexKey::ParamDeclaration { name: request.name.clone() },
        // `auto_derived` is never trusted from the request — only `auto_declare_params` may set it.
        DataValue::ParamDeclaration(ParamDeclaration { auto_derived: false, ..request }),
    );
    Ok(())
}

/// Turns an arbitrary log-line key into a valid regex named-capture-group identifier (ASCII
/// alphanumeric/underscore, not starting with a digit) — the param's own `name` (shown to the user,
/// used as the index key) keeps the raw key text; only the capture group embedded in its generated
/// regex needs sanitizing. Two different keys sanitizing to the same group name is harmless since
/// each param's regex/capture-group pair is private to that one param.
fn sanitize_group_name(key: &str) -> String {
    let mut group: String =
        key.chars().map(|c| if c.is_ascii_alphanumeric() || c == '_' { c } else { '_' }).collect();
    if group.chars().next().is_none_or(|c| c.is_ascii_digit()) {
        group.insert(0, '_');
    }
    group
}

/// Conservative kind inference for an auto-derived param's first sample: only `Number` is guessed
/// (int, falling back to float), everything else stays `String` — `Boolean`/`Timestamp`/`Duration`
/// carry extraction semantics beyond "parses as", so guessing them wrong would silently drop values
/// that don't fit (see `parse_metric_primitive`).
fn infer_kind(sample: &str) -> MetricPrimitiveKind {
    if sample.parse::<i64>().is_ok() || sample.parse::<f64>().is_ok() {
        MetricPrimitiveKind::Number
    } else {
        MetricPrimitiveKind::String
    }
}

/// Builds the regex an auto-derived param uses to re-extract its value from any line on read
/// (`derive_log_params`) — quoted keys always look for a `"..."` value, unquoted keys a
/// whitespace-delimited bareword, matching how `crate::logfmt::extract_pairs` found the sample.
fn build_auto_param_pattern(key: &str, group: &str, quoted: bool) -> String {
    let escaped_key = regex::escape(key);
    if quoted {
        format!(r#"{escaped_key}="(?P<{group}>[^"]*)""#)
    } else {
        format!(r"{escaped_key}=(?P<{group}>\S+)")
    }
}

/// Hard ceiling on how many params `auto_declare_params` will ever create automatically over the
/// life of the engine. Without this, one unusual input can permanently inflate the declared-param
/// set forever — `auto_declare_params` never overwrites or prunes a declaration, so every distinct
/// key ever seen sticks around. Ticket 050's real `log stream` dump discovered 220 distinct keys from
/// under 700 lines, mostly one-off WiFi/radio stat counters (e.g. `txRTSFrm`/`rxCTSUcast`/...) that
/// will never recur meaningfully. Chosen alongside `MAX_NEW_AUTO_PARAMS_PER_LINE` (below) rather than
/// on its own: a per-line cap alone still lets an unbounded *variety* of lines each contribute a few
/// new keys forever, so this total ceiling is the actual backstop.
const MAX_AUTO_DECLARED_PARAMS: usize = 64;

/// Per-line ceiling on how many *new* params a single ingested line may contribute to the auto-derived
/// set. Without this, one unusually wide line (the fixture's WiFi stat dump has 20+ `key=value` pairs
/// in a single line) could burn most of `MAX_AUTO_DECLARED_PARAMS` in one shot, crowding out keys from
/// more representative lines ingested later. Pairs beyond the cap are simply never declared (from this
/// line or any other, since a later line with fewer pairs would still need to reach them within its own
/// per-line budget) — acceptable since they're exactly the low-value, one-off counters ticket 050 flags.
const MAX_NEW_AUTO_PARAMS_PER_LINE: usize = 8;

fn auto_derived_param_count(index: &BTreeMap<IndexKey, DataValue>) -> usize {
    param_declarations_range(index).filter(|declaration| declaration.6).count()
}

/// One param worth auto-declaring, found on an ingested line by either extraction path
/// `auto_declare_params` supports: a logfmt `key=value` pair (name, and a regex/capture-group built
/// from it) or a JSON leaf field (name being its dot-separated path, extracted via `json_field_path`
/// instead of a regex). Lets `auto_declare_params` share one insert-and-budget loop across both.
struct AutoParamCandidate {
    name: String,
    value: String,
    message_match_regex: String,
    capture_group: String,
    json_field_path: Option<String>,
}

/// Scans `text` for auto-derivable params and declares a standalone param for any name not already
/// declared — auto or manual. A manual declaration for a name always wins and is never touched here;
/// this only fills in names nobody has declared yet. JSON lines (`crate::logfmt::detect_format`) are
/// scanned via `crate::data_value::flatten_json_object` (dot-path leaf fields, extracted by
/// `json_field_path` rather than regex, since a `key=value` scan finds nothing useful in JSON's
/// `"key":value` shape); anything else falls back to the original `key=value` pair scan
/// (`crate::logfmt::extract_pairs`). Run on every ingested line (`EngineMessage::Ingest`), same as
/// `apply_all_metrics`, but note param *values* are still recomputed from raw text at read time
/// (`derive_log_params`), not stored here — so a key discovered on line N also back-fills the column
/// for lines before N on the next read. Bounded by `MAX_AUTO_DECLARED_PARAMS`/
/// `MAX_NEW_AUTO_PARAMS_PER_LINE` (see their doc comments) so a single burst of unusual lines can't
/// permanently inflate the declared-param set (ticket 050).
fn auto_declare_params(index: &mut BTreeMap<IndexKey, DataValue>, text: &str) {
    let mut total = auto_derived_param_count(index);
    if total >= MAX_AUTO_DECLARED_PARAMS {
        return;
    }
    let candidates: Vec<AutoParamCandidate> = if crate::logfmt::detect_format(text) == "json" {
        crate::data_value::flatten_json_object(text)
            .into_iter()
            .map(|(path, value)| AutoParamCandidate {
                name: path.clone(),
                value,
                message_match_regex: String::new(),
                capture_group: String::new(),
                json_field_path: Some(path),
            })
            .collect()
    } else {
        crate::logfmt::extract_pairs(text)
            .into_iter()
            .map(|(key, value, quoted)| {
                let group = sanitize_group_name(key);
                AutoParamCandidate {
                    name: key.to_string(),
                    value: value.to_string(),
                    message_match_regex: build_auto_param_pattern(key, &group, quoted),
                    capture_group: group,
                    json_field_path: None,
                }
            })
            .collect()
    };

    let mut new_this_line = 0;
    for candidate in candidates {
        if new_this_line >= MAX_NEW_AUTO_PARAMS_PER_LINE || total >= MAX_AUTO_DECLARED_PARAMS {
            break;
        }
        let index_key = IndexKey::ParamDeclaration { name: candidate.name.clone() };
        if index.contains_key(&index_key) {
            continue;
        }
        index.insert(
            index_key,
            DataValue::ParamDeclaration(ParamDeclaration {
                name: candidate.name,
                message_match_regex: candidate.message_match_regex,
                capture_group: candidate.capture_group,
                json_field_path: candidate.json_field_path,
                kind: infer_kind(&candidate.value),
                elapsed_since_last: false,
                auto_derived: true,
            }),
        );
        new_this_line += 1;
        total += 1;
    }
}

/// Declares one param per column of the most-recently-seen `log stream` header line, plus a trailing
/// unlabelled `message` param for whatever text follows the last named column (ticket 059) — replacing
/// the old fixed three (`log_type`/`process`/`pid`) with whatever columns the header actually names.
/// `header` is threaded through by the caller (`run`'s ingest loop) rather than owned here: a
/// `log stream` pipe prints its header once, before any data line, and this function only ever sees
/// one ingested line at a time, so the header has to be cached across calls somewhere. A later header
/// line (e.g. a fresh `log stream` invocation piped into the same run) simply replaces the cached one.
/// Unlike `auto_declare_params`, adding these is never *skipped* on account of
/// `MAX_AUTO_DECLARED_PARAMS` — though since they're stored as `auto_derived` too, once present they do
/// count against that shared ceiling, tightening (slightly) the budget left for generic `key=value`
/// pairs. Like `auto_declare_params`, never overwrites an existing declaration (manual or auto) for
/// any of these names.
fn auto_declare_log_stream_params(
    index: &mut BTreeMap<IndexKey, DataValue>,
    header: &mut Option<crate::logfmt::LogStreamHeader>,
    text: &str,
) {
    if let Some(detected) = crate::logfmt::detect_log_stream_header(text) {
        *header = Some(detected);
        return;
    }
    let Some(header) = header.as_ref() else {
        return;
    };
    if !crate::logfmt::is_log_stream_line(text) {
        return;
    }
    let row = crate::logfmt::split_log_stream_row(text, header);

    for (i, column) in header.columns.iter().enumerate() {
        let Some(value) = row.get(i) else { continue };
        let index_key = IndexKey::ParamDeclaration { name: column.name.clone() };
        if index.contains_key(&index_key) {
            continue;
        }
        index.insert(
            index_key,
            DataValue::ParamDeclaration(ParamDeclaration {
                name: column.name.clone(),
                message_match_regex: crate::logfmt::log_stream_column_pattern(header, i, &column.name),
                capture_group: column.name.clone(),
                json_field_path: None,
                kind: infer_kind(value.trim()),
                elapsed_since_last: false,
                auto_derived: true,
            }),
        );
    }

    if let Some(message) = row.get(header.columns.len()).filter(|m| !m.trim().is_empty()) {
        index.entry(IndexKey::ParamDeclaration { name: "message".to_string() }).or_insert_with(|| {
            DataValue::ParamDeclaration(ParamDeclaration {
                name: "message".to_string(),
                message_match_regex: crate::logfmt::log_stream_message_pattern(header, "message"),
                capture_group: "message".to_string(),
                json_field_path: None,
                kind: infer_kind(message.trim()),
                elapsed_since_last: false,
                auto_derived: true,
            })
        });
    }
}

/// Ticket 052: removes every `IndexKey::ParamDeclaration` entry — manual and auto-derived alike.
/// Returns how many were removed. Unlike `RemoveParamDeclaration`'s single-name removal (used by
/// `reload_engine_declarations` during a profile switch, which deliberately keeps auto-derived
/// params since they're engine-discovered facts rather than profile state), this is a full reset:
/// the caller (`EngineMessage::ClearParams`) immediately re-runs `auto_declare_params` afterward, so
/// wiping the auto-derived ones too just means they come back fresh instead of stale.
fn clear_all_param_declarations(index: &mut BTreeMap<IndexKey, DataValue>) -> usize {
    let names: Vec<String> =
        param_declarations_range(index).map(|(name, ..)| name.to_string()).collect();
    let removed = names.len();
    for name in names {
        index.remove(&IndexKey::ParamDeclaration { name });
    }
    removed
}

/// Ticket 052: re-runs `auto_declare_params` against every `IndexKey::IngestedLog` entry still held
/// in the index, oldest to newest (matching normal ingest order, so declaration-order semantics —
/// e.g. which key "wins" a naming collision — come out the same as if the lines were freshly
/// ingested). Only ever called right after `clear_all_param_declarations`, so every param produced
/// here is fresh, not a re-declaration.
///
/// Returns `(reextracted, logs_considered)`: how many params came back, and how many retained log
/// lines were re-scanned. Lines already dropped by `evict_excess_logs` before this ran aren't in the
/// index anymore and so can't contribute — `logs_considered` is how the caller surfaces that bound
/// to the user, rather than the re-extraction silently looking complete when it isn't.
fn reextract_params_from_retained_logs(index: &mut BTreeMap<IndexKey, DataValue>) -> (usize, usize) {
    let texts: Vec<String> = logs_range(index).map(|(_, _, text)| text.to_string()).collect();
    let logs_considered = texts.len();
    for text in &texts {
        auto_declare_params(index, text);
    }
    let reextracted = param_declarations_range(index).count();
    (reextracted, logs_considered)
}

/// Validates the metric's own regex plus that every name in `label_params`/`value_param` refers to
/// an already-declared standalone param (not just the first bad one, so a rejection reports every
/// problem at once). Shared by `declare_metric` and `update_metric` so both reject a bad
/// declaration the same way.
fn validate_metric_request(
    index: &BTreeMap<IndexKey, DataValue>,
    request: &DeclareMetricRequest,
) -> Result<Regex, String> {
    let re = Regex::new(&request.message_match_regex)
        .map_err(|err| format!("invalid regex: {err}"))?;

    let declared = |name: &str| index.contains_key(&IndexKey::ParamDeclaration { name: name.to_string() });

    let mut errors: Vec<String> = request
        .label_params
        .iter()
        .filter(|name| !declared(name))
        .map(|name| format!("label param '{name}' references an undeclared param"))
        .collect();
    if let Some(value_param) = &request.value_param {
        if !declared(value_param) {
            errors.push(format!("value param '{value_param}' references an undeclared param"));
        }
    }
    if !errors.is_empty() {
        return Err(errors.join("; "));
    }

    Ok(re)
}

fn declare_metric(
    index: &mut BTreeMap<IndexKey, DataValue>,
    request: DeclareMetricRequest,
) -> Result<(), String> {
    let re = validate_metric_request(index, &request)?;
    let resolved_params = resolved_param_declarations(index);

    backfill_metric(
        index,
        &request.name,
        &re,
        &request.label_params,
        request.value_param.as_deref(),
        &resolved_params,
        request.matched_labels.as_ref(),
        request.unmatched_labels.as_ref(),
    );
    index.insert(
        IndexKey::MetricDeclaration { name: request.name },
        DataValue::MetricDeclaration {
            message_match_regex: request.message_match_regex,
            label_params: request.label_params,
            value_param: request.value_param,
            matched_labels: request.matched_labels,
            unmatched_labels: request.unmatched_labels,
        },
    );
    Ok(())
}

/// Updates the metric declaration named `existing_name` in place, reusing `declare_metric`'s validation so an edit is rejected the same way a bad new declaration would be. Fails (rather than creating a fresh declaration) if no metric named `existing_name` exists yet. Existing bucket/last-event data for this metric is cleared first and rebuilt from the full log history under the new regex/params, so edited matchers don't leave stale buckets from the previous declaration mixed in with fresh ones. Supports renaming (when `request.name` differs from `existing_name`) as long as the new name isn't already taken by another declaration.
fn update_metric(
    index: &mut BTreeMap<IndexKey, DataValue>,
    existing_name: &str,
    request: DeclareMetricRequest,
) -> Result<(), String> {
    if !index.contains_key(&IndexKey::MetricDeclaration { name: existing_name.to_string() }) {
        return Err(format!("no metric declaration named '{existing_name}'"));
    }
    if request.name != existing_name
        && index.contains_key(&IndexKey::MetricDeclaration { name: request.name.clone() })
    {
        return Err(format!("a metric named '{}' already exists", request.name));
    }

    let re = validate_metric_request(index, &request)?;
    let resolved_params = resolved_param_declarations(index);

    clear_metric_data(index, existing_name);
    if request.name != existing_name {
        index.remove(&IndexKey::MetricDeclaration { name: existing_name.to_string() });
    }

    backfill_metric(
        index,
        &request.name,
        &re,
        &request.label_params,
        request.value_param.as_deref(),
        &resolved_params,
        request.matched_labels.as_ref(),
        request.unmatched_labels.as_ref(),
    );
    index.insert(
        IndexKey::MetricDeclaration { name: request.name.clone() },
        DataValue::MetricDeclaration {
            message_match_regex: request.message_match_regex,
            label_params: request.label_params,
            value_param: request.value_param,
            matched_labels: request.matched_labels,
            unmatched_labels: request.unmatched_labels,
        },
    );
    Ok(())
}

/// Removes every `Bucket` and `LastMetricEvent` entry belonging to `metric_name`, ahead of a full re-backfill in `update_metric` — otherwise the old declaration's buckets would sit alongside freshly backfilled ones under the same metric name.
fn clear_metric_data(index: &mut BTreeMap<IndexKey, DataValue>, metric_name: &str) {
    index.retain(|key, _| match key {
        IndexKey::Bucket { metric_name: name, .. } => name != metric_name,
        IndexKey::LastMetricEvent { metric_name: name, .. } => name != metric_name,
        _ => true,
    });
}

/// Removes `name`'s declaration and, per ticket 051, purges its bucket history along with it
/// (reusing `clear_metric_data`, same as `update_metric`'s re-backfill does for the old
/// declaration) rather than leaving it orphaned — an orphaned bucket would let a stale dashboard
/// panel or PromQL query keep silently returning old data by name instead of failing with a clear
/// "unknown metric" error (see the declared-or-has-data check in `evaluate_promql`). Returns
/// whether a declaration existed, which the caller maps to 404 vs success at the API layer.
fn remove_metric_declaration(index: &mut BTreeMap<IndexKey, DataValue>, name: &str) -> bool {
    let existed = index.remove(&IndexKey::MetricDeclaration { name: name.to_string() }).is_some();
    clear_metric_data(index, name);
    existed
}

/// Returns `None` on a parse failure, which the caller treats as "this match doesn't apply".
fn parse_metric_primitive(raw: &str, kind: MetricPrimitiveKind) -> Option<MetricPrimitive> {
    match kind {
        MetricPrimitiveKind::String => Some(MetricPrimitive::String(raw.to_string())),
        MetricPrimitiveKind::Number => raw
            .parse::<i64>()
            .map(Number::Int)
            .or_else(|_| raw.parse::<f64>().map(Number::Float))
            .ok()
            .map(MetricPrimitive::Number),
        MetricPrimitiveKind::Timestamp => {
            crate::filter_datetime::parse_datetime(raw, Timestamp::now())
                .ok()
                .map(MetricPrimitive::Timestamp)
        }
        // An `elapsed_since_last` param never reaches this branch (see `resolve_param_value`, which
        // computes its value directly rather than extracting/parsing text); reached only for a
        // Duration-kind param that parses a literal captured/field millisecond count.
        MetricPrimitiveKind::Duration => raw.parse::<i64>().ok().map(MetricPrimitive::Duration),
        // A standalone Boolean param never reaches this branch (see derive_log_params, which derives its
        // value from whether the regex matched rather than parsing captured text); reached only if a metric
        // matcher references a Boolean-kind param, where it's parsed as a literal "true"/"false" capture.
        MetricPrimitiveKind::Boolean => raw.parse::<bool>().ok().map(MetricPrimitive::Boolean),
    }
}

/// A `ParamDeclaration` with its regex pre-compiled, keyed by name — resolved once per ingest/backfill
/// pass (not once per metric) so metrics sharing a param don't each recompile its regex. A param
/// whose stored regex fails to compile (shouldn't happen — `declare_param` validates it) is omitted
/// rather than panicking, same as `compiled_param_declarations`.
struct ResolvedParam {
    regex: Regex,
    capture_group: String,
    json_field_path: Option<String>,
    kind: MetricPrimitiveKind,
    /// See `crate::data_value::ParamDeclaration::elapsed_since_last`. When set, `regex`/
    /// `capture_group`/`json_field_path` above are unused — resolution goes through
    /// `resolve_param_value`'s computed path instead of `extract_param_raw`.
    elapsed_since_last: bool,
}

fn resolved_param_declarations(index: &BTreeMap<IndexKey, DataValue>) -> HashMap<String, ResolvedParam> {
    param_declarations_range(index)
        .filter_map(|(name, message_match_regex, capture_group, json_field_path, kind, elapsed_since_last, _)| {
            Regex::new(message_match_regex).ok().map(|regex| {
                (
                    name.to_string(),
                    ResolvedParam {
                        regex,
                        capture_group: capture_group.to_string(),
                        json_field_path: json_field_path.map(|path| path.to_string()),
                        kind,
                        elapsed_since_last,
                    },
                )
            })
        })
        .collect()
}

/// Extracts a param's raw captured text from `text` via its own regex (JSON field path taking
/// precedence when set) — shared by `apply_metric_to_log` (resolving a metric's `label_params`/
/// `value_param`) and `derive_log_params` (a standalone param declaration shown as a logs-table
/// column). Returns `None` when the regex doesn't match, or (for a JSON field path) the field can't
/// be resolved.
fn extract_param_raw(
    text: &str,
    regex: &Regex,
    capture_group: &str,
    json_field_path: Option<&str>,
) -> Option<String> {
    match json_field_path.filter(|path| !path.is_empty()) {
        Some(field_path) => crate::data_value::extract_json_field(text, field_path),
        None => regex.captures(text)?.name(capture_group).map(|m| m.as_str().to_string()),
    }
}

/// Resolves one param's value against `text`/`timestamp`, dispatching purely on what the param
/// itself declares (`ResolvedParam::elapsed_since_last`) rather than on whether it's being resolved
/// as a label or as the value — the same resolution is used for both in `apply_metric_to_log`, so
/// an `elapsed_since_last` param behaves identically regardless of which list references it. An
/// elapsed-since-last param's value is elapsed time since the previous event that produced
/// `labels_so_far` under `metric_name`, tracked via a `LastMetricEvent` index entry; its first
/// occurrence returns `None` (nothing to diff against yet) rather than a bogus zero. Every other
/// param falls through to ordinary text extraction (regex capture group or JSON field path) parsed
/// as its declared kind.
fn resolve_param_value(
    index: &mut BTreeMap<IndexKey, DataValue>,
    metric_name: &str,
    resolved: &ResolvedParam,
    labels_so_far: &BTreeMap<String, MetricPrimitive>,
    text: &str,
    timestamp: Timestamp,
) -> Option<MetricPrimitive> {
    if resolved.elapsed_since_last {
        // Tracked separately from Bucket: the first occurrence for a given label set pushes no
        // Bucket entry, so the last-event timestamp can't live in Bucket.values.
        let last_event_key = IndexKey::LastMetricEvent {
            metric_name: metric_name.to_string(),
            labels: labels_so_far.clone(),
        };
        let previous_timestamp = match index.get(&last_event_key) {
            Some(DataValue::LastEventTimestamp { timestamp: previous }) => Some(*previous),
            _ => None,
        };
        index.insert(last_event_key, DataValue::LastEventTimestamp { timestamp });
        return previous_timestamp
            .map(|previous| MetricPrimitive::Duration(timestamp.as_millis() - previous.as_millis()));
    }

    let raw = extract_param_raw(text, &resolved.regex, &resolved.capture_group, resolved.json_field_path.as_deref())?;
    parse_metric_primitive(&raw, resolved.kind)
}

/// Resolves `label_params`/`value_param` by name against `resolved_params` — a dangling reference
/// (a param removed after this metric was declared) skips the whole match, same as any other
/// extraction failure. At most one param is the value param (its primitive becomes the event's
/// value, otherwise a fixed `1` marker is used); any param failing to match/parse skips the whole
/// match rather than storing a partial one. When `regex` (the metric's own message-match regex)
/// doesn't match and `unmatched_labels` is `Some`, an occurrence is still registered with those
/// fixed labels.
#[allow(clippy::too_many_arguments)]
fn apply_metric_to_log(
    index: &mut BTreeMap<IndexKey, DataValue>,
    metric_name: &str,
    regex: &Regex,
    label_params: &[String],
    value_param: Option<&str>,
    resolved_params: &HashMap<String, ResolvedParam>,
    matched_labels: Option<&BTreeMap<String, String>>,
    unmatched_labels: Option<&BTreeMap<String, String>>,
    sequence: u64,
    timestamp: Timestamp,
    text: &str,
) {
    if regex.captures(text).is_none() {
        if let Some(unmatched_labels) = unmatched_labels {
            let labels = unmatched_labels
                .iter()
                .map(|(name, value)| (name.clone(), MetricPrimitive::String(value.clone())))
                .collect();
            push_bucket_value(index, metric_name, labels, sequence, timestamp, MetricPrimitive::Number(Number::Int(1)));
        }
        return;
    }

    let mut labels = BTreeMap::new();
    for param_name in label_params {
        let Some(resolved) = resolved_params.get(param_name) else { return };
        let Some(parsed) = resolve_param_value(index, metric_name, resolved, &labels, text, timestamp) else {
            return;
        };
        labels.insert(param_name.clone(), parsed);
    }

    if let Some(matched_labels) = matched_labels {
        for (name, value) in matched_labels {
            labels.insert(name.clone(), MetricPrimitive::String(value.clone()));
        }
    }

    let value = match value_param {
        Some(value_param) => {
            let Some(resolved) = resolved_params.get(value_param) else { return };
            match resolve_param_value(index, metric_name, resolved, &labels, text, timestamp) {
                Some(parsed) => parsed,
                None => return,
            }
        }
        None => MetricPrimitive::Number(Number::Int(1)),
    };

    push_bucket_value(index, metric_name, labels, sequence, timestamp, value);
}

fn push_bucket_value(
    index: &mut BTreeMap<IndexKey, DataValue>,
    metric_name: &str,
    labels: BTreeMap<String, MetricPrimitive>,
    sequence: u64,
    timestamp: Timestamp,
    value: MetricPrimitive,
) {
    let key = IndexKey::Bucket { metric_name: metric_name.to_string(), labels };
    let entry = index
        .entry(key)
        .or_insert_with(|| DataValue::BucketValue { values: Vec::new() });
    if let DataValue::BucketValue { values } = entry {
        values.push((sequence, timestamp, value));
    }
}

/// Backfills buckets for a newly-declared metric against every already-ingested log, not just future ones.
#[allow(clippy::too_many_arguments)]
fn backfill_metric(
    index: &mut BTreeMap<IndexKey, DataValue>,
    metric_name: &str,
    regex: &Regex,
    label_params: &[String],
    value_param: Option<&str>,
    resolved_params: &HashMap<String, ResolvedParam>,
    matched_labels: Option<&BTreeMap<String, String>>,
    unmatched_labels: Option<&BTreeMap<String, String>>,
) {
    let lines: Vec<(u64, Timestamp, String)> = logs_range(index)
        .map(|(sequence, timestamp, text)| (sequence, timestamp, text.to_string()))
        .collect();
    for (sequence, timestamp, text) in lines {
        apply_metric_to_log(
            index,
            metric_name,
            regex,
            label_params,
            value_param,
            resolved_params,
            matched_labels,
            unmatched_labels,
            sequence,
            timestamp,
            &text,
        );
    }
}

fn apply_all_metrics(
    index: &mut BTreeMap<IndexKey, DataValue>,
    sequence: u64,
    timestamp: Timestamp,
    text: &str,
) {
    type SeededMetric = (
        String,
        String,
        Vec<String>,
        Option<String>,
        Option<BTreeMap<String, String>>,
        Option<BTreeMap<String, String>>,
    );
    let metrics: Vec<SeededMetric> = metric_declarations_range(index)
        .map(|(name, message_match_regex, label_params, value_param, matched_labels, unmatched_labels)| {
            (
                name.to_string(),
                message_match_regex.to_string(),
                label_params.to_vec(),
                value_param.map(|s| s.to_string()),
                matched_labels.cloned(),
                unmatched_labels.cloned(),
            )
        })
        .collect();
    // Resolved once per ingested line (shared across every metric that runs against it), not once per metric.
    let resolved_params = resolved_param_declarations(index);
    for (name, message_match_regex, label_params, value_param, matched_labels, unmatched_labels) in metrics {
        if let Ok(re) = Regex::new(&message_match_regex) {
            apply_metric_to_log(
                index,
                &name,
                &re,
                &label_params,
                value_param.as_deref(),
                &resolved_params,
                matched_labels.as_ref(),
                unmatched_labels.as_ref(),
                sequence,
                timestamp,
                text,
            );
        }
    }
}

/// The metrics API layers its own textual label-filter and range filtering on top rather than using this function's `labels_filter` param (used directly by tests).
fn bucket_values<'a>(
    index: &'a BTreeMap<IndexKey, DataValue>,
    metric_name: &'a str,
    labels_filter: Option<&'a BTreeMap<String, MetricPrimitive>>,
) -> impl Iterator<Item = (&'a BTreeMap<String, MetricPrimitive>, &'a (u64, Timestamp, MetricPrimitive))> + 'a
{
    index
        .range(IndexKey::MetricBucketSplit..IndexKey::End)
        .filter_map(move |(key, value)| match (key, value) {
            (IndexKey::Bucket { metric_name: name, labels }, DataValue::BucketValue { values })
                if name == metric_name && labels_filter.is_none_or(|filter| filter == labels) =>
            {
                Some(values.iter().map(move |entry| (labels, entry)))
            }
            _ => None,
        })
        .flatten()
}

/// A [`FilterExpr`] compiled against a snapshot of the index, so evaluating per log line touches the index only once: a `metric[param]` comparison is pre-resolved into a `HashSet` of matching sequences ([`CompiledFilter::MetricMatch`]) for O(1) per-line lookup instead of a regex re-run; a comparison with no `[param]` is rejected since v1 buckets have no single "the metric's value", only a per-label-combo occurrence marker.
enum CompiledFilter {
    Regex(Regex),
    Sequence { op: CompareOp, target: f64 },
    Timestamp { op: CompareOp, target_ms: i64 },
    /// `ottyl_fmt <op> value` — like `Json`, this is line-local and re-detected fresh per line in
    /// `eval_compiled` (`crate::logfmt::detect_format`) rather than pre-resolved against the index.
    Fmt { op: CompareOp, target: String },
    MetricMatch(HashSet<u64>),
    /// `json[field] <op> value` — unlike `MetricMatch`, this can't be pre-resolved against the
    /// index (the JSON lives in the log line itself, not a declared metric/param), so it's
    /// evaluated per line in `eval_compiled` by parsing `text` as JSON.
    Json { field: String, op: CompareOp, value: FilterValue },
    And(Box<CompiledFilter>, Box<CompiledFilter>),
    Or(Box<CompiledFilter>, Box<CompiledFilter>),
}

/// Resolves every regex/metric-comparison/datetime-literal up front so per-line evaluation ([`eval_compiled`]) never touches the index; `now` anchors relative literals so every comparison in one query resolves against the same instant.
fn compile_filter_query(
    index: &BTreeMap<IndexKey, DataValue>,
    filter: &str,
    now: Timestamp,
) -> Result<CompiledFilter, String> {
    let expr = filter_parser::parse(filter).map_err(|err| err.to_string())?;
    compile_filter(index, &expr, now)
}

fn compile_filter(
    index: &BTreeMap<IndexKey, DataValue>,
    expr: &FilterExpr,
    now: Timestamp,
) -> Result<CompiledFilter, String> {
    match expr {
        FilterExpr::Regex(pattern) => Regex::new(pattern)
            .map(CompiledFilter::Regex)
            .map_err(|err| format!("invalid regex in filter: {err}")),
        FilterExpr::Comparison { metric, param, op, value } => match metric.as_str() {
            "ottyl_seq" => compile_ottyl_seq_comparison(*op, value),
            "ottyl_ts" => compile_ottyl_ts_comparison(*op, value, now),
            "ottyl_fmt" => Ok(compile_ottyl_fmt_comparison(*op, value)),
            other => compile_metric_comparison(index, other, param.as_deref(), *op, value, now),
        },
        FilterExpr::JsonComparison { field, op, value } => Ok(CompiledFilter::Json {
            field: field.clone(),
            op: *op,
            value: value.clone(),
        }),
        FilterExpr::And(lhs, rhs) => Ok(CompiledFilter::And(
            Box::new(compile_filter(index, lhs, now)?),
            Box::new(compile_filter(index, rhs, now)?),
        )),
        FilterExpr::Or(lhs, rhs) => Ok(CompiledFilter::Or(
            Box::new(compile_filter(index, lhs, now)?),
            Box::new(compile_filter(index, rhs, now)?),
        )),
    }
}

/// A `Raw` value is accepted defensively (parsed as a number) even though the grammar shouldn't produce one for `ottyl_seq`.
fn compile_ottyl_seq_comparison(op: CompareOp, value: &FilterValue) -> Result<CompiledFilter, String> {
    let target = match value {
        FilterValue::Number(n) => *n,
        FilterValue::Raw(raw) => raw.parse::<f64>().map_err(|_| {
            format!("'ottyl_seq' comparisons require a numeric value, got {raw:?}")
        })?,
    };
    Ok(CompiledFilter::Sequence { op, target })
}

/// A `Number` is treated as raw milliseconds; a `Raw` token is resolved via `filter_datetime::parse_datetime` against `now`.
fn compile_ottyl_ts_comparison(
    op: CompareOp,
    value: &FilterValue,
    now: Timestamp,
) -> Result<CompiledFilter, String> {
    let target_ms = match value {
        FilterValue::Number(n) => *n as i64,
        FilterValue::Raw(raw) => filter_datetime::parse_datetime(raw, now)
            .map_err(|err| err.to_string())?
            .as_millis(),
    };
    Ok(CompiledFilter::Timestamp { op, target_ms })
}

/// A `Number` target is stringified since `ottyl_fmt`'s value ("json"/"logfmt"/"none",
/// `crate::logfmt::detect_format`) is always text; any op is accepted since the strings compare
/// lexicographically fine for `==`/`!=` (the only ops that make real sense here), same as
/// `compare_metric_primitive`'s `String` arm.
fn compile_ottyl_fmt_comparison(op: CompareOp, value: &FilterValue) -> CompiledFilter {
    let target = match value {
        FilterValue::Number(n) => n.to_string(),
        FilterValue::Raw(raw) => raw.clone(),
    };
    CompiledFilter::Fmt { op, target }
}

/// `metric[param] <op> value` for any metric name other than `ottyl_seq`/`ottyl_ts`/`ottyl_fmt` — see [`CompiledFilter`]'s doc comment.
fn compile_metric_comparison(
    index: &BTreeMap<IndexKey, DataValue>,
    metric: &str,
    param: Option<&str>,
    op: CompareOp,
    value: &FilterValue,
    now: Timestamp,
) -> Result<CompiledFilter, String> {
    let param = param.ok_or_else(|| {
        format!(
            "metric comparison on '{metric}' requires a [param] (e.g. '{metric}[some_param]{}...'); \
             v1 buckets only carry per-label occurrence data, not a single metric value",
            match op {
                CompareOp::Lt => "<",
                CompareOp::Gt => ">",
                CompareOp::Le => "<=",
                CompareOp::Ge => ">=",
                CompareOp::Eq => "==",
                CompareOp::Ne => "!=",
            }
        )
    })?;

    // Validate `param` against the metric's actual declared `label_params` (ticket 037's unified
    // `MetricDeclaration`) rather than just reading whatever key happens to be present in a
    // bucket's `labels` map — a typo or a param that was never declared as a label on this metric
    // would otherwise silently match nothing instead of being rejected up front. A metric name
    // with no declaration at all is left alone here (`bucket_values` below simply yields nothing
    // for it, matching prior behavior) since there's no label_params list to validate against.
    if let Some(DataValue::MetricDeclaration { label_params, .. }) =
        index.get(&IndexKey::MetricDeclaration { name: metric.to_string() })
    {
        if !label_params.iter().any(|p| p == param) {
            return Err(format!(
                "param '{param}' is not a label param of metric '{metric}'; declared label params: [{}]",
                label_params.join(", ")
            ));
        }
    }

    let mut matching = HashSet::new();
    for (labels, (sequence, _, _)) in bucket_values(index, metric, None) {
        let Some(primitive) = labels.get(param) else {
            continue;
        };
        if compare_metric_primitive(primitive, op, value, now)? {
            matching.insert(*sequence);
        }
    }
    Ok(CompiledFilter::MetricMatch(matching))
}

/// Mismatched kinds that can't be meaningfully compared (e.g. a string label against a bare number) simply never match, rather than erroring.
fn compare_metric_primitive(
    primitive: &MetricPrimitive,
    op: CompareOp,
    value: &FilterValue,
    now: Timestamp,
) -> Result<bool, String> {
    let ordering = match (primitive, value) {
        (MetricPrimitive::Number(n), FilterValue::Number(v)) => n.as_f64().partial_cmp(v),
        (MetricPrimitive::Number(n), FilterValue::Raw(raw)) => {
            let v: f64 = raw
                .parse()
                .map_err(|_| format!("expected a numeric value, got {raw:?}"))?;
            n.as_f64().partial_cmp(&v)
        }
        (MetricPrimitive::Timestamp(ts), FilterValue::Number(v)) => {
            ts.as_millis().partial_cmp(&(*v as i64))
        }
        (MetricPrimitive::Timestamp(ts), FilterValue::Raw(raw)) => {
            let target = filter_datetime::parse_datetime(raw, now).map_err(|err| err.to_string())?;
            ts.as_millis().partial_cmp(&target.as_millis())
        }
        (MetricPrimitive::String(s), FilterValue::Raw(raw)) => s.as_str().partial_cmp(raw.as_str()),
        (MetricPrimitive::String(s), FilterValue::Number(n)) => {
            s.as_str().partial_cmp(n.to_string().as_str())
        }
        (MetricPrimitive::Duration(ms), FilterValue::Number(v)) => (*ms as f64).partial_cmp(v),
        (MetricPrimitive::Duration(ms), FilterValue::Raw(raw)) => {
            let v: f64 = raw
                .parse()
                .map_err(|_| format!("expected a numeric value, got {raw:?}"))?;
            (*ms as f64).partial_cmp(&v)
        }
        (MetricPrimitive::Boolean(b), FilterValue::Raw(raw)) => {
            let v: bool = raw
                .parse()
                .map_err(|_| format!("expected true or false, got {raw:?}"))?;
            b.partial_cmp(&v)
        }
        (MetricPrimitive::Boolean(_), FilterValue::Number(_)) => None,
    };
    Ok(ordering.is_some_and(|ordering| apply_compare_op(ordering, op)))
}

/// `json[field] <op> value`: the log line is parsed as JSON fresh on every call (no per-line
/// cache), since this doesn't go through the index-backed pre-resolution `MetricMatch` uses.
/// Any line that isn't a JSON object, or lacks `field`, simply doesn't match — no error.
pub(crate) fn eval_json_comparison(text: &str, field: &str, op: CompareOp, value: &FilterValue) -> bool {
    let Ok(parsed) = serde_json::from_str::<serde_json::Value>(text) else {
        return false;
    };
    let Some(field_value) = parsed.as_object().and_then(|obj| obj.get(field)) else {
        return false;
    };
    compare_json_value(field_value, op, value)
}

/// Mirrors `compare_metric_primitive`'s "mismatched kinds never match" behavior. Only `Eq`/`Ne`
/// are meaningful for booleans, and `null`/array/object fields aren't scalar-comparable at all.
fn compare_json_value(field_value: &serde_json::Value, op: CompareOp, value: &FilterValue) -> bool {
    match field_value {
        serde_json::Value::Number(n) => {
            let target = match value {
                FilterValue::Number(v) => Some(*v),
                FilterValue::Raw(raw) => raw.parse::<f64>().ok(),
            };
            match (n.as_f64(), target) {
                (Some(f), Some(v)) => f.partial_cmp(&v).is_some_and(|ordering| apply_compare_op(ordering, op)),
                _ => false,
            }
        }
        serde_json::Value::String(s) => {
            let target = match value {
                FilterValue::Raw(raw) => raw.clone(),
                FilterValue::Number(n) => n.to_string(),
            };
            s.as_str()
                .partial_cmp(target.as_str())
                .is_some_and(|ordering| apply_compare_op(ordering, op))
        }
        serde_json::Value::Bool(b) => {
            let FilterValue::Raw(raw) = value else {
                return false;
            };
            match raw.parse::<bool>() {
                Ok(v) => match op {
                    CompareOp::Eq => *b == v,
                    CompareOp::Ne => *b != v,
                    _ => false,
                },
                Err(_) => false,
            }
        }
        serde_json::Value::Null | serde_json::Value::Array(_) | serde_json::Value::Object(_) => false,
    }
}

/// `pub(crate)` so `crate::alerts` can reuse it for `Threshold`/`Ratio` comparisons instead of duplicating the match.
pub(crate) fn apply_compare_op(ordering: Ordering, op: CompareOp) -> bool {
    match op {
        CompareOp::Lt => ordering == Ordering::Less,
        CompareOp::Gt => ordering == Ordering::Greater,
        CompareOp::Le => ordering != Ordering::Greater,
        CompareOp::Ge => ordering != Ordering::Less,
        CompareOp::Eq => ordering == Ordering::Equal,
        CompareOp::Ne => ordering != Ordering::Equal,
    }
}

/// Cheap: everything was already resolved by [`compile_filter_query`], so this is just pattern matching and `HashSet` lookups — no index access.
fn eval_compiled(compiled: &CompiledFilter, sequence: u64, timestamp: Timestamp, text: &str) -> bool {
    match compiled {
        CompiledFilter::Regex(re) => re.is_match(text),
        CompiledFilter::Sequence { op, target } => {
            apply_compare_op((sequence as f64).total_cmp(target), *op)
        }
        CompiledFilter::Timestamp { op, target_ms } => {
            apply_compare_op(timestamp.as_millis().cmp(target_ms), *op)
        }
        CompiledFilter::Fmt { op, target } => {
            apply_compare_op(crate::logfmt::detect_format(text).cmp(target.as_str()), *op)
        }
        CompiledFilter::MetricMatch(sequences) => sequences.contains(&sequence),
        CompiledFilter::Json { field, op, value } => eval_json_comparison(text, field, *op, value),
        CompiledFilter::And(lhs, rhs) => {
            eval_compiled(lhs, sequence, timestamp, text) && eval_compiled(rhs, sequence, timestamp, text)
        }
        CompiledFilter::Or(lhs, rhs) => {
            eval_compiled(lhs, sequence, timestamp, text) || eval_compiled(rhs, sequence, timestamp, text)
        }
    }
}

fn metric_declarations_range(
    index: &BTreeMap<IndexKey, DataValue>,
) -> impl Iterator<
    Item = (
        &str,
        &str,
        &[String],
        Option<&str>,
        Option<&BTreeMap<String, String>>,
        Option<&BTreeMap<String, String>>,
    ),
> {
    index
        .range(IndexKey::ParamMetricSplit..IndexKey::MetricBucketSplit)
        .filter_map(|(key, value)| match (key, value) {
            (
                IndexKey::MetricDeclaration { name },
                DataValue::MetricDeclaration {
                    message_match_regex,
                    label_params,
                    value_param,
                    matched_labels,
                    unmatched_labels,
                },
            ) => Some((
                name.as_str(),
                message_match_regex.as_str(),
                label_params.as_slice(),
                value_param.as_deref(),
                matched_labels.as_ref(),
                unmatched_labels.as_ref(),
            )),
            _ => None,
        })
}

fn param_declarations_range(
    index: &BTreeMap<IndexKey, DataValue>,
) -> impl Iterator<Item = (&str, &str, &str, Option<&str>, MetricPrimitiveKind, bool, bool)> {
    index
        .range(IndexKey::LogParamSplit..IndexKey::ParamMetricSplit)
        .filter_map(|(key, value)| match (key, value) {
            (
                IndexKey::ParamDeclaration { name },
                DataValue::ParamDeclaration(ParamDeclaration {
                    name: _,
                    message_match_regex,
                    capture_group,
                    json_field_path,
                    kind,
                    elapsed_since_last,
                    auto_derived,
                }),
            ) => Some((
                name.as_str(),
                message_match_regex.as_str(),
                capture_group.as_str(),
                json_field_path.as_deref(),
                *kind,
                *elapsed_since_last,
                *auto_derived,
            )),
            _ => None,
        })
}

/// A param whose stored regex fails to compile (shouldn't happen — `declare_param` validates it) is
/// skipped rather than panicking. An `elapsed_since_last` param is skipped outright — it has no
/// meaningful reading as a standalone logs-table column (its value depends on which metric's label
/// set it's grouped under, which only exists once it's referenced by a metric's `label_params`/
/// `value_param`).
/// Ticket 060: key type for a sort helper index — one such index exists per param name that's ever
/// been used as a `sort_by`. Declaration order matters for the derived `Ord`: every `WithParam` key
/// sorts before every `WithoutParam` key, and within each variant ordering falls out of the tuple
/// fields (value then sequence; sequence alone), which is exactly `sort_missing_fallback`'s "sorted
/// lines first, then lines missing the param ordered by `ottyl_seq`" semantics — expressed as part of
/// the index's own ordering instead of a separate partition-and-append step per query.
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
enum SortHelperKey {
    WithParam(MetricPrimitive, u64),
    WithoutParam(u64),
}

/// One param's helper index plus when a query last actually read from it — ticket 061 sweeps entries
/// whose `last_accessed` is stale. The map's value is that line's `Timestamp`, not `()`, purely so a
/// query can reconstruct the exact `IndexKey::IngestedLog { sequence, timestamp }` primary-index key
/// and look its text up in O(log n) instead of re-scanning `logs_range` to find it by sequence alone.
struct SortHelperIndexEntry {
    index: BTreeMap<SortHelperKey, Timestamp>,
    last_accessed: std::time::Instant,
}

/// How long a sort helper index may sit unread before ticket 061's sweep drops it. Not exposed via
/// `Settings` (ticket 061 explicitly scopes that out) — a dropped index is always transparently
/// rebuildable on the next query for that sort key, so this is purely a memory-bound knob, not a
/// user-visible behavior change.
const SORT_HELPER_INDEX_PURGE_AFTER: std::time::Duration = std::time::Duration::from_secs(300);

/// Derives a single named param's value from `text`, for helper-index maintenance where deriving
/// every declared param (`derive_log_params`) per ingested line would be wasted work when only one
/// sort key's helper index actually needs updating.
fn derive_single_param(
    text: &str,
    declarations: &[(String, Regex, String, Option<String>, MetricPrimitiveKind)],
    param_name: &str,
) -> Option<MetricPrimitive> {
    let (_, regex, capture_group, json_field_path, kind) =
        declarations.iter().find(|(name, ..)| name == param_name)?;
    if *kind == MetricPrimitiveKind::Boolean {
        return Some(MetricPrimitive::Boolean(regex.is_match(text)));
    }
    let captures = regex.captures(text)?;
    let raw: String = match json_field_path.as_deref().filter(|path| !path.is_empty()) {
        Some(field_path) => crate::data_value::extract_json_field(text, field_path)?,
        None => captures.name(capture_group)?.as_str().to_string(),
    };
    parse_metric_primitive(&raw, *kind)
}

/// One-time full scan building a sort helper index for `sort_by` from every currently-ingested log —
/// used both the first time a param is sorted by (no helper index exists yet) and to transparently
/// rebuild one ticket 061's sweep purged.
fn build_sort_helper_index(index: &BTreeMap<IndexKey, DataValue>, sort_by: &str) -> BTreeMap<SortHelperKey, Timestamp> {
    let compiled_params = compiled_param_declarations(index);
    logs_range(index)
        .map(|(sequence, timestamp, text)| {
            let key = match derive_single_param(text, &compiled_params, sort_by) {
                Some(value) => SortHelperKey::WithParam(value, sequence),
                None => SortHelperKey::WithoutParam(sequence),
            };
            (key, timestamp)
        })
        .collect()
}

/// Incrementally extends every currently-live helper index with one freshly-ingested line, called
/// from `EngineMessage::Ingest` right alongside the primary index insert (ticket 060) — this is what
/// lets a sorted query avoid re-deriving params for the whole timeline on every call once a helper
/// index for that sort key exists. A no-op when no helper index has been built yet (the common case
/// for a param nobody has sorted by), so ingest pays nothing for sort keys that are never queried.
fn update_sort_helper_indexes_on_ingest(
    sort_helper_indexes: &mut HashMap<String, SortHelperIndexEntry>,
    index: &BTreeMap<IndexKey, DataValue>,
    sequence: u64,
    timestamp: Timestamp,
    text: &str,
) {
    if sort_helper_indexes.is_empty() {
        return;
    }
    let compiled_params = compiled_param_declarations(index);
    for (sort_by, entry) in sort_helper_indexes.iter_mut() {
        let key = match derive_single_param(text, &compiled_params, sort_by) {
            Some(value) => SortHelperKey::WithParam(value, sequence),
            None => SortHelperKey::WithoutParam(sequence),
        };
        entry.index.insert(key, timestamp);
    }
}

/// Ticket 061: drops any helper index whose `last_accessed` exceeds `SORT_HELPER_INDEX_PURGE_AFTER`.
/// Cheap to call opportunistically on every ingest (`HashMap::retain` is a no-op scan when nothing's
/// stale) rather than needing a dedicated timer/task — a purged index rebuilds transparently via
/// `build_sort_helper_index` the next time something sorts by it.
fn purge_stale_sort_helper_indexes(sort_helper_indexes: &mut HashMap<String, SortHelperIndexEntry>) {
    sort_helper_indexes.retain(|_, entry| entry.last_accessed.elapsed() < SORT_HELPER_INDEX_PURGE_AFTER);
}

fn compiled_param_declarations(
    index: &BTreeMap<IndexKey, DataValue>,
) -> Vec<(String, Regex, String, Option<String>, MetricPrimitiveKind)> {
    param_declarations_range(index)
        .filter(|(_, _, _, _, _, elapsed_since_last, _)| !elapsed_since_last)
        .filter_map(|(name, message_match_regex, capture_group, json_field_path, kind, _, _)| {
            Regex::new(message_match_regex).ok().map(|re| {
                (
                    name.to_string(),
                    re,
                    capture_group.to_string(),
                    json_field_path.map(|path| path.to_string()),
                    kind,
                )
            })
        })
        .collect()
}

/// A param is omitted (not included as null/empty) when its regex/capture group (or JSON field
/// path) doesn't match, or the extracted text fails to parse — see `LogEntryDto::params`. The one
/// exception is a `Boolean`-kind param, which never uses a capture group and is instead present as
/// `true`/`false` on every line based purely on whether `message_match_regex` matches
/// (case-insensitively or per the regex's own flags).
fn derive_log_params(
    text: &str,
    declarations: &[(String, Regex, String, Option<String>, MetricPrimitiveKind)],
) -> BTreeMap<String, MetricPrimitive> {
    let mut params = BTreeMap::new();
    for (name, regex, capture_group, json_field_path, kind) in declarations {
        if *kind == MetricPrimitiveKind::Boolean {
            params.insert(name.clone(), MetricPrimitive::Boolean(regex.is_match(text)));
            continue;
        }
        let Some(captures) = regex.captures(text) else {
            continue;
        };
        let raw: String = match json_field_path.as_deref().filter(|path| !path.is_empty()) {
            Some(field_path) => match crate::data_value::extract_json_field(text, field_path) {
                Some(value) => value,
                None => continue,
            },
            None => match captures.name(capture_group) {
                Some(m) => m.as_str().to_string(),
                None => continue,
            },
        };
        let Some(value) = parse_metric_primitive(&raw, *kind) else {
            continue;
        };
        params.insert(name.clone(), value);
    }
    params
}

/// Ticket 011's approximate per-log byte size: see `StatsDto::approx_bytes_total`'s
/// doc comment for why the raw UTF-8 text length is the chosen proxy rather than a
/// full struct-size estimate.
fn approx_log_bytes(text: &str) -> u64 {
    text.len() as u64
}

/// Evicts the oldest (lowest-`sequence`) ingested logs until `log_count` is
/// back at or under `max_logs`. `sequence` numbering is not renumbered on
/// eviction, and any `Bucket` values already derived from an evicted log are
/// left in place — buckets are aggregate data, not raw logs, so they're not
/// retroactively pruned. `approx_bytes_total` (ticket 011) is decremented by
/// each evicted log's approximate size, mirroring the increment on ingest.
fn evict_excess_logs(
    index: &mut BTreeMap<IndexKey, DataValue>,
    log_count: &mut usize,
    max_logs: Option<usize>,
    approx_bytes_total: &mut u64,
) {
    let Some(max_logs) = max_logs else { return };
    while *log_count > max_logs {
        let Some(oldest_key) = index
            .range(IndexKey::Start..IndexKey::LogParamSplit)
            .next()
            .map(|(key, _)| key.clone())
        else {
            break;
        };
        if let Some(DataValue::LogLiteral { text }) = index.get(&oldest_key) {
            *approx_bytes_total = approx_bytes_total.saturating_sub(approx_log_bytes(text));
        }
        index.remove(&oldest_key);
        *log_count -= 1;
    }
}

fn logs_range(index: &BTreeMap<IndexKey, DataValue>) -> impl Iterator<Item = (u64, Timestamp, &str)> {
    index.range(IndexKey::Start..IndexKey::LogParamSplit).filter_map(|(key, value)| {
        match (key, value) {
            (IndexKey::IngestedLog { sequence, timestamp }, DataValue::LogLiteral { text }) => {
                Some((*sequence, *timestamp, text.as_str()))
            }
            _ => None,
        }
    })
}

/// Every currently-ingested log with `sequence < before_sequence` and timestamp in
/// `(since_ms, now_ms]`, in ascending order. `pub(crate)` so `crate::alerts` can use it to
/// backfill a freshly-created `Threshold`/`Ratio` window from already-ingested history — bounded
/// to the alert's own window, not the full log history, so the cost stays proportional to the
/// window rather than the whole ingest history (ticket 040).
pub(crate) fn logs_since(
    index: &BTreeMap<IndexKey, DataValue>,
    before_sequence: u64,
    since_ms: i64,
    now_ms: i64,
) -> impl Iterator<Item = (u64, Timestamp, &str)> {
    logs_range(index).filter(move |(sequence, timestamp, _)| {
        *sequence < before_sequence
            && timestamp.as_millis() > since_ms
            && timestamp.as_millis() <= now_ms
    })
}

fn handle_query(
    index: &BTreeMap<IndexKey, DataValue>,
    query: EngineQuery,
    approx_bytes_total: u64,
    sort_helper_indexes: &mut HashMap<String, SortHelperIndexEntry>,
) -> EngineResponse {
    match query {
        EngineQuery::Logs(params) => {
            let compiled_filter = match params.filter.as_deref() {
                Some(filter) => match compile_filter_query(index, filter, Timestamp::now()) {
                    Ok(compiled) => Some(compiled),
                    Err(err) => return EngineResponse::Error(err),
                },
                None => None,
            };
            // Ticket 045: `sort_by` must name an already-declared param — reject anything else up
            // front rather than silently falling through to "every line is missing it".
            if let Some(sort_by) = params.sort_by.as_deref() {
                if !param_declarations_range(index).any(|(name, ..)| name == sort_by) {
                    return EngineResponse::Error(format!("unknown param '{sort_by}' for sort_by"));
                }
            }
            let limit = params.limit.unwrap_or(500).min(10_000);
            // O(lines_returned * declared_params) per query — fine at v1 scale; if volume/params grow, move to incremental derivation at ingest time like apply_all_metrics does for buckets.
            let compiled_params = compiled_param_declarations(index);
            let filtered = logs_range(index)
                .filter(|(sequence, timestamp, _)| {
                    params.since_sequence.is_none_or(|since| *sequence > since)
                        && params.start_ms.is_none_or(|start| timestamp.as_millis() >= start)
                        && params.end_ms.is_none_or(|end| timestamp.as_millis() <= end)
                })
                .filter(|(sequence, timestamp, text)| {
                    compiled_filter
                        .as_ref()
                        .is_none_or(|filter| eval_compiled(filter, *sequence, *timestamp, text))
                });

            let entries: Vec<LogEntryDto> = match params.sort_by.as_deref() {
                // Default path: unchanged from pre-045 behavior — take(limit) before mapping, so
                // only the entries actually returned pay the per-line param-derivation cost.
                None => filtered
                    .take(limit)
                    .map(|(sequence, timestamp, text)| LogEntryDto {
                        sequence,
                        timestamp_ms: timestamp.as_millis(),
                        text: text.to_string(),
                        params: derive_log_params(text, &compiled_params),
                    })
                    .collect(),
                // Ticket 060: walk the (lazily built-or-reused) helper index for this sort key in
                // key order instead of materializing and sorting the full filtered set — the index's
                // own key ordering already expresses "sorted-by-value lines, then lines missing the
                // param ordered by ottyl_seq" (`sort_missing_fallback`'s semantics), so this just
                // filters/limits as it walks rather than partition-sort-concat afterward.
                Some(sort_by) => {
                    let helper = sort_helper_indexes.entry(sort_by.to_string()).or_insert_with(|| {
                        SortHelperIndexEntry {
                            index: build_sort_helper_index(index, sort_by),
                            last_accessed: std::time::Instant::now(),
                        }
                    });
                    helper.last_accessed = std::time::Instant::now();

                    let sort_missing_fallback = params.sort_missing_fallback.unwrap_or(true);
                    let mut entries = Vec::new();
                    for (helper_key, timestamp) in helper.index.iter() {
                        let sequence = match helper_key {
                            SortHelperKey::WithParam(_, sequence) => *sequence,
                            SortHelperKey::WithoutParam(sequence) => {
                                if !sort_missing_fallback {
                                    continue;
                                }
                                *sequence
                            }
                        };
                        if !(params.since_sequence.is_none_or(|since| sequence > since)
                            && params.start_ms.is_none_or(|start| timestamp.as_millis() >= start)
                            && params.end_ms.is_none_or(|end| timestamp.as_millis() <= end))
                        {
                            continue;
                        }
                        // A line evicted (by `max_logs`) since this key was recorded is simply
                        // skipped, same as buckets/other derived data aren't retroactively pruned
                        // on eviction elsewhere in this file.
                        let Some(DataValue::LogLiteral { text }) =
                            index.get(&IndexKey::IngestedLog { sequence, timestamp: *timestamp })
                        else {
                            continue;
                        };
                        if !compiled_filter
                            .as_ref()
                            .is_none_or(|filter| eval_compiled(filter, sequence, *timestamp, text))
                        {
                            continue;
                        }
                        entries.push(LogEntryDto {
                            sequence,
                            timestamp_ms: timestamp.as_millis(),
                            text: text.clone(),
                            params: derive_log_params(text, &compiled_params),
                        });
                        if entries.len() >= limit {
                            break;
                        }
                    }
                    entries
                }
            };
            let entries: Vec<LogEntryDto> = entries.into_iter().take(limit).collect();
            EngineResponse::Logs(entries)
        }
        EngineQuery::Timeline(params) => {
            let compiled_filter = match params.filter.as_deref() {
                Some(filter) => match compile_filter_query(index, filter, Timestamp::now()) {
                    Ok(compiled) => Some(compiled),
                    Err(err) => return EngineResponse::Error(err),
                },
                None => None,
            };
            let bucket_ms = params.bucket_ms.unwrap_or(1_000).max(1);
            let mut buckets: BTreeMap<i64, u64> = BTreeMap::new();
            for (sequence, timestamp, text) in logs_range(index).filter(|(_, timestamp, _)| {
                params.start_ms.is_none_or(|start| timestamp.as_millis() >= start)
                    && params.end_ms.is_none_or(|end| timestamp.as_millis() <= end)
            }) {
                if compiled_filter
                    .as_ref()
                    .is_some_and(|filter| !eval_compiled(filter, sequence, timestamp, text))
                {
                    continue;
                }
                let bucket_start = (timestamp.as_millis() / bucket_ms) * bucket_ms;
                *buckets.entry(bucket_start).or_insert(0) += 1;
            }
            let out = buckets
                .into_iter()
                .map(|(start_ms, count)| TimelineBucketDto { start_ms, count })
                .collect();
            EngineResponse::Timeline(out)
        }
        EngineQuery::Stats => {
            let mut log_count = 0u64;
            let mut first_sequence = None;
            let mut last_sequence = None;
            let mut first_timestamp_ms = None;
            let mut last_timestamp_ms = None;
            for (sequence, timestamp, _) in logs_range(index) {
                log_count += 1;
                first_sequence.get_or_insert(sequence);
                last_sequence = Some(sequence);
                first_timestamp_ms.get_or_insert(timestamp.as_millis());
                last_timestamp_ms = Some(timestamp.as_millis());
            }
            EngineResponse::Stats(StatsDto {
                log_count,
                first_sequence,
                last_sequence,
                first_timestamp_ms,
                last_timestamp_ms,
                approx_bytes_total,
            })
        }
        EngineQuery::ParamDeclarations => {
            let declarations = param_declarations_range(index)
                .map(|(name, message_match_regex, capture_group, json_field_path, kind, elapsed_since_last, auto_derived)| {
                    ParamDeclarationDto {
                        name: name.to_string(),
                        message_match_regex: message_match_regex.to_string(),
                        capture_group: capture_group.to_string(),
                        json_field_path: json_field_path.map(|path| path.to_string()),
                        kind,
                        elapsed_since_last,
                        auto_derived,
                        reserved: false,
                    }
                })
                .collect();
            EngineResponse::ParamDeclarations(declarations)
        }
        EngineQuery::MetricDeclarations => {
            let declarations = metric_declarations_range(index)
                .map(|(name, message_match_regex, label_params, value_param, matched_labels, unmatched_labels)| {
                    MetricDeclarationDto {
                        name: name.to_string(),
                        message_match_regex: message_match_regex.to_string(),
                        label_params: label_params.to_vec(),
                        value_param: value_param.map(|s| s.to_string()),
                        matched_labels: matched_labels.cloned(),
                        unmatched_labels: unmatched_labels.cloned(),
                    }
                })
                .collect();
            EngineResponse::MetricDeclarations(declarations)
        }
        EngineQuery::BucketValues {
            metric_name,
            labels_filter,
            start_ms,
            end_ms,
            since_sequence,
            limit,
        } => {
            let limit = limit.unwrap_or(500).min(10_000);
            let values: Vec<BucketValueDto> = bucket_values(index, &metric_name, None)
                .filter(|(labels, _)| {
                    labels_filter.as_ref().is_none_or(|filter| {
                        filter.iter().all(|(name, expected)| {
                            labels
                                .get(name)
                                .is_some_and(|value| &value.to_string() == expected)
                        })
                    })
                })
                .filter(|(_, (sequence, timestamp, _))| {
                    since_sequence.is_none_or(|since| *sequence > since)
                        && start_ms.is_none_or(|start| timestamp.as_millis() >= start)
                        && end_ms.is_none_or(|end| timestamp.as_millis() <= end)
                })
                .take(limit)
                .map(|(labels, (sequence, timestamp, value))| BucketValueDto {
                    labels: labels.clone(),
                    sequence: *sequence,
                    timestamp_ms: timestamp.as_millis(),
                    value: value.clone(),
                })
                .collect();
            EngineResponse::BucketValues(values)
        }
        EngineQuery::Promql { query, start_ms, end_ms } => {
            let response = promql::parse_query(&query)
                .and_then(|parsed| evaluate_promql(index, &parsed, start_ms, end_ms));
            EngineResponse::Promql(response)
        }
    }
}

/// Coerces a bucket's `MetricPrimitive` into a plottable `f64`: Number/Timestamp/Duration use their natural numeric value; `String` has no numeric reading, so it falls back to `1.0` (matching the presence/occurrence marker used for every bucket) rather than being skipped, keeping every declared metric queryable as a bare series or under `rate()`/`gauge()`, which only ever count or sample points rather than summing this value. `sum()`/`avg()` do read it, so `evaluate_promql` rejects those aggregations up front for a `String`-kind metric (see `declared_value_kind`) rather than letting this 1.0 stand in for a sum.
fn coerce_to_f64(value: &MetricPrimitive) -> f64 {
    match value {
        MetricPrimitive::Number(n) => match n {
            Number::Int(v) => *v as f64,
            Number::Float(v) => *v,
        },
        MetricPrimitive::Timestamp(ts) => ts.as_millis() as f64,
        MetricPrimitive::Duration(ms) => *ms as f64,
        MetricPrimitive::String(_) => 1.0,
        MetricPrimitive::Boolean(b) => {
            if *b {
                1.0
            } else {
                0.0
            }
        }
    }
}

/// Compared against each label's `Display` form; extra labels on the bucket are ignored — the matcher list only needs to be a subset.
fn matches_label_matchers(
    labels: &BTreeMap<String, MetricPrimitive>,
    matchers: &[LabelMatcher],
) -> bool {
    matchers.iter().all(|matcher| {
        labels.get(&matcher.name).is_some_and(|value| value.to_string() == matcher.value)
    })
}

/// Projects a full label map down to just the names in `by_labels`; an empty `by_labels` collapses
/// everything to the empty map, i.e. "one group total" (the historical `sum()`/`avg()` behavior).
fn project_labels(
    labels: &BTreeMap<String, String>,
    by_labels: &[String],
) -> BTreeMap<String, String> {
    by_labels
        .iter()
        .filter_map(|name| labels.get(name).map(|value| (name.clone(), value.clone())))
        .collect()
}

/// Thin adapter over the single shared windowed-rate implementation (`crate::rate`, ticket 040):
/// PromQL's series here are plain millisecond `i64` timestamps rather than `Timestamp`, so this
/// just converts at the boundary and delegates — the actual window-scan logic lives only in
/// `crate::rate::windowed_rate_series`, also used (via `crate::rate::RateWindow`) by alert
/// `Threshold`/`Ratio` conditions in `crate::alerts`, so both agree on what "windowed rate" means.
///
/// Ticket 057: `rate::windowed_rate_series` only evaluates *at each occurrence's own timestamp*,
/// so a metric whose occurrences stop (or start) well inside a much wider queried range produces a
/// series that just ends (or begins) at the last (or first) real occurrence — nothing says "the
/// rate is 0 here" for the rest of the range. When `start_ms`/`end_ms` are given (a dashboard panel
/// always supplies both), this appends a synthetic boundary point at each edge — computed via
/// `rate::rate_at` against the original occurrence points, so a boundary that falls inside a
/// still-warm window reads its true partial value rather than being forced to 0 — whenever the
/// real series doesn't already reach that edge.
fn windowed_rate_series(
    points: &[(i64, f64)],
    range_ms: i64,
    start_ms: Option<i64>,
    end_ms: Option<i64>,
) -> Vec<(i64, f64)> {
    let ts_points: Vec<(Timestamp, f64)> =
        points.iter().map(|(ms, value)| (Timestamp::from_millis(*ms), *value)).collect();
    let mut series: Vec<(i64, f64)> = rate::windowed_rate_series(&ts_points, range_ms)
        .into_iter()
        .map(|(ts, value)| (ts.as_millis(), value))
        .collect();

    if let Some(end) = end_ms {
        if series.last().is_none_or(|(ts, _)| *ts < end) {
            let value = rate::rate_at(&ts_points, Timestamp::from_millis(end), range_ms);
            series.push((end, value));
        }
    }
    if let Some(start) = start_ms {
        if series.first().is_none_or(|(ts, _)| *ts > start) {
            let value = rate::rate_at(&ts_points, Timestamp::from_millis(start), range_ms);
            series.insert(0, (start, value));
        }
    }
    series
}

/// Fixed window width (ms) used by `merge_series_windowed` to combine multiple raw-occurrence
/// sub-series into one time series — the same fixed-width bucketing style `histogram()`/the
/// `Timeline` query already use elsewhere in this file, just applied along the time axis instead of
/// the value axis. Occurrence timestamps across different label groups (e.g. different hosts serving
/// the same path) essentially never land on the same millisecond, so summing/averaging only requires
/// matching *this* window to combine into "total over time" rather than going sparse (ticket 042).
const MERGE_WINDOW_MS: i64 = 1_000;

/// Merges several series into one combined time series (ticket 042). A single sub-series is returned
/// untouched — there's only one contributor, so there's nothing to merge and no reason to lose
/// resolution. With two or more sub-series, points are bucketed into fixed `MERGE_WINDOW_MS`-wide
/// windows (rather than requiring an exact-millisecond timestamp match) before summing/averaging
/// within each window, so raw occurrence markers from different label groups combine into a
/// meaningful "total over time" instead of each window reflecting only whichever sub-series happened
/// to report at that exact millisecond.
fn merge_series_windowed(sub_series: &[Vec<(i64, f64)>], is_avg: bool) -> Vec<(i64, f64)> {
    if sub_series.len() <= 1 {
        return sub_series.first().cloned().unwrap_or_default();
    }
    let mut grouped: BTreeMap<i64, Vec<f64>> = BTreeMap::new();
    for points in sub_series {
        for (timestamp_ms, value) in points {
            let window_start = timestamp_ms.div_euclid(MERGE_WINDOW_MS) * MERGE_WINDOW_MS;
            grouped.entry(window_start).or_default().push(*value);
        }
    }
    grouped
        .into_iter()
        .map(|(timestamp_ms, values)| {
            let sum: f64 = values.iter().sum();
            let value = if is_avg { sum / values.len() as f64 } else { sum };
            (timestamp_ms, value)
        })
        .collect()
}

/// Merges several already-windowed rate series by sweeping the union of their timestamps, carrying each
/// sub-series' last-known value forward (0.0 before its first point) so every step reflects every
/// sub-series' most recent reading — unlike `merge_series_windowed`, which buckets by fixed time windows instead.
fn merge_series_locf(sub_series: &[Vec<(i64, f64)>], is_avg: bool) -> Vec<(i64, f64)> {
    let mut all_timestamps: std::collections::BTreeSet<i64> = std::collections::BTreeSet::new();
    for points in sub_series {
        all_timestamps.extend(points.iter().map(|(ts, _)| *ts));
    }
    let mut cursors = vec![0usize; sub_series.len()];
    let mut last_values = vec![0.0f64; sub_series.len()];
    all_timestamps
        .into_iter()
        .map(|ts| {
            for (i, points) in sub_series.iter().enumerate() {
                while cursors[i] < points.len() && points[cursors[i]].0 <= ts {
                    last_values[i] = points[cursors[i]].1;
                    cursors[i] += 1;
                }
            }
            let sum: f64 = last_values.iter().sum();
            let value = if is_avg { sum / last_values.len() as f64 } else { sum };
            (ts, value)
        })
        .collect()
}

/// The value kind a metric's buckets carry: the kind of its `Value`-role param, or `Number` when
/// it has none (every occurrence then falls back to a fixed `Number::Int(1)` marker — see
/// `apply_metric_to_log`). Returns `None` when `metric_name` has no declaration at all, in which
/// case there's nothing to validate against and the query simply yields no series.
fn declared_value_kind(
    index: &BTreeMap<IndexKey, DataValue>,
    metric_name: &str,
) -> Option<MetricPrimitiveKind> {
    match index.get(&IndexKey::MetricDeclaration { name: metric_name.to_string() }) {
        Some(DataValue::MetricDeclaration { value_param, .. }) => Some(
            value_param
                .as_ref()
                .and_then(|name| {
                    index.get(&IndexKey::ParamDeclaration { name: name.clone() })
                })
                .map(|value| match value {
                    DataValue::ParamDeclaration(param) => param.kind,
                    _ => MetricPrimitiveKind::Number,
                })
                .unwrap_or(MetricPrimitiveKind::Number),
        ),
        _ => None,
    }
}

/// The three reserved param names usable without declaring anything — see `compile_ottyl_seq_comparison`/
/// `compile_ottyl_ts_comparison`/`compile_ottyl_fmt_comparison` for the filter-bar equivalent, and
/// `webapp/src/components/Docs.tsx` for the user-facing docs. Shared by `evaluate_promql` (dashboard
/// panels, this file) and `crate::api::list_params` (`GET /api/params` discoverability).
pub const OTTYL_RESERVED_PARAM_NAMES: [&str; 3] = ["ottyl_seq", "ottyl_ts", "ottyl_fmt"];

/// For `ottyl_seq`/`ottyl_ts` only (not `ottyl_fmt`, which `evaluate_promql` rejects outright before
/// this is consulted): returns a `(sequence, timestamp_ms) -> value` mapping used to synthesize a
/// virtual PromQL series straight from the ingested-log index, since neither is backed by a
/// `MetricDeclaration`/bucket the way every other metric name is.
fn ottyl_reserved_numeric_value_fn(metric_name: &str) -> Option<fn(u64, i64) -> f64> {
    match metric_name {
        "ottyl_seq" => Some(|sequence, _timestamp_ms| sequence as f64),
        "ottyl_ts" => Some(|_sequence, timestamp_ms| timestamp_ms as f64),
        _ => None,
    }
}

/// Evaluates a parsed PromQL query against `index`, producing chart-ready series: baseline is one series per label combination; `sum()`/`avg()` merge series into one series per `by_labels` group (the whole result collapses to a single label-less series when `by_labels` is empty); `rate()` — bare or wrapped in `sum()`/`avg()` — turns each series into a windowed occurrences-per-second reading via `windowed_rate_series`, so it plots as a real line rather than one trailing point; `histogram()` groups by `by_labels` the same way sum()/avg() do, then buckets each group's values independently, so `histogram(m) by (label)` yields one histogram (a family of `le`-labeled series carrying the group's own labels) per label group instead of collapsing all groups together.
/// `sum()`/`avg()` over a `String`-kind metric are rejected up front — `coerce_to_f64` has no
/// numeric reading for a string, so silently treating every occurrence as `1.0` would let a sum
/// pass for a count with no signal to the caller. `sum(rate(...))`/`avg(rate(...))` are exempt:
/// the inner `rate()` only counts occurrences per window and never reads the coerced value, so a
/// string-kind metric under `rate()` is already the count semantic the ticket asks for.
fn evaluate_promql(
    index: &BTreeMap<IndexKey, DataValue>,
    parsed: &PromqlQuery,
    start_ms: Option<i64>,
    end_ms: Option<i64>,
) -> Result<Vec<PromqlSeriesDto>, String> {
    // `ottyl_fmt` is categorical ("json"/"logfmt"/"none", `crate::logfmt::detect_format`) with no
    // sensible numeric reading, unlike `ottyl_seq`/`ottyl_ts` below — rather than silently
    // returning an empty series (the failure mode ticket 055 calls out), a dashboard query over it
    // is rejected up front with a message pointing back at the filter/query bar, where it already
    // works (see `compile_ottyl_fmt_comparison`).
    if parsed.metric_name == "ottyl_fmt" {
        return Err(
            "'ottyl_fmt' is a categorical value (json/logfmt/none), not a number, so it can't be \
             used in a dashboard PromQL query; filter on it in the search bar instead, e.g. \
             ottyl_fmt==json"
                .to_string(),
        );
    }

    // Ticket 051: a metric with neither a declaration nor any recorded bucket data is genuinely
    // unknown (as opposed to a declared-but-empty-so-far metric, or an undeclared scraped
    // Prometheus metric ingested via `--metrics-url`'s `IngestMetricSample`, which never gets a
    // `MetricDeclaration` but still has bucket data) — most commonly because a dashboard panel
    // still references a metric that has since been deleted. Failing here with a clear error beats
    // a panic or a silently-empty result. `ottyl_seq`/`ottyl_ts` (ticket 055) are exempt: they're
    // synthesized straight from the ingested-log index below and never have a `MetricDeclaration`
    // or bucket entries of their own.
    if ottyl_reserved_numeric_value_fn(&parsed.metric_name).is_none()
        && !index.contains_key(&IndexKey::MetricDeclaration { name: parsed.metric_name.clone() })
        && bucket_values(index, &parsed.metric_name, None).next().is_none()
    {
        return Err(format!(
            "unknown metric \"{}\": no declaration or recorded data for it (was it deleted?)",
            parsed.metric_name
        ));
    }
    if matches!(parsed.agg, Some(AggOp::Sum) | Some(AggOp::Avg))
        && !parsed.inner_rate
        && declared_value_kind(index, &parsed.metric_name) == Some(MetricPrimitiveKind::String)
    {
        return Err(format!(
            "cannot sum/avg a string-typed metric \"{}\"; use count() or a numeric-typed metric",
            parsed.metric_name
        ));
    }

    let mut series_points: BTreeMap<BTreeMap<String, String>, Vec<(i64, f64)>> = BTreeMap::new();
    if let Some(reserved_value) = ottyl_reserved_numeric_value_fn(&parsed.metric_name) {
        // `ottyl_seq`/`ottyl_ts` are computed line-locally (see `compile_ottyl_seq_comparison`/
        // `compile_ottyl_ts_comparison`) rather than backed by a `MetricDeclaration` and bucket
        // entries, so they're synthesized here straight from the ingested-log index instead of
        // going through `bucket_values`. Neither carries any labels, so a non-empty matcher list
        // simply matches nothing, same as an unmatched label filter would against a real metric.
        let no_labels: BTreeMap<String, MetricPrimitive> = BTreeMap::new();
        if matches_label_matchers(&no_labels, &parsed.matchers) {
            for (sequence, timestamp, _text) in logs_range(index) {
                let timestamp_ms = timestamp.as_millis();
                if start_ms.is_none_or(|start| timestamp_ms >= start)
                    && end_ms.is_none_or(|end| timestamp_ms <= end)
                {
                    series_points
                        .entry(BTreeMap::new())
                        .or_default()
                        .push((timestamp_ms, reserved_value(sequence, timestamp_ms)));
                }
            }
        }
    } else {
        for (labels, (_, timestamp, value)) in bucket_values(index, &parsed.metric_name, None)
            .filter(|(labels, _)| matches_label_matchers(labels, &parsed.matchers))
            .filter(|(_, (_, timestamp, _))| {
                start_ms.is_none_or(|start| timestamp.as_millis() >= start)
                    && end_ms.is_none_or(|end| timestamp.as_millis() <= end)
            })
        {
            let display_labels: BTreeMap<String, String> =
                labels.iter().map(|(name, value)| (name.clone(), value.to_string())).collect();
            series_points
                .entry(display_labels)
                .or_default()
                .push((timestamp.as_millis(), coerce_to_f64(value)));
        }
    }
    for points in series_points.values_mut() {
        points.sort_by_key(|(timestamp_ms, _)| *timestamp_ms);
    }

    Ok(match parsed.agg {
        None => series_points
            .into_iter()
            .map(|(labels, points)| PromqlSeriesDto { labels, points })
            .collect(),
        Some(AggOp::Count) => {
            // Occurrence count per group, ignoring each point's coerced value entirely — the
            // explicit opt-in for the count-shaped result `sum()`/`avg()` used to give a
            // `String`-kind metric by accident (see `coerce_to_f64`).
            let mut groups: BTreeMap<BTreeMap<String, String>, Vec<Vec<(i64, f64)>>> = BTreeMap::new();
            for (labels, points) in series_points {
                let count_points: Vec<(i64, f64)> =
                    points.iter().map(|(timestamp_ms, _)| (*timestamp_ms, 1.0)).collect();
                let group_key = project_labels(&labels, &parsed.by_labels);
                groups.entry(group_key).or_default().push(count_points);
            }
            groups
                .into_iter()
                .map(|(group_labels, sub_series)| {
                    let points = merge_series_windowed(&sub_series, false);
                    PromqlSeriesDto { labels: group_labels, points }
                })
                .collect()
        }
        Some(AggOp::Sum) | Some(AggOp::Avg) => {
            let is_avg = parsed.agg == Some(AggOp::Avg);
            // Validated by `promql::parse_query`: an inner rate() always carries a range.
            let rate_range_ms = parsed
                .inner_rate
                .then(|| parsed.range.expect("inner rate() always carries a range").as_millis() as i64);

            let mut groups: BTreeMap<BTreeMap<String, String>, Vec<Vec<(i64, f64)>>> = BTreeMap::new();
            for (labels, points) in series_points {
                let points = match rate_range_ms {
                    Some(range_ms) => windowed_rate_series(&points, range_ms, start_ms, end_ms),
                    None => points,
                };
                let group_key = project_labels(&labels, &parsed.by_labels);
                groups.entry(group_key).or_default().push(points);
            }

            groups
                .into_iter()
                .map(|(group_labels, sub_series)| {
                    let points = if rate_range_ms.is_some() {
                        merge_series_locf(&sub_series, is_avg)
                    } else {
                        merge_series_windowed(&sub_series, is_avg)
                    };
                    PromqlSeriesDto { labels: group_labels, points }
                })
                .collect()
        }
        Some(AggOp::Rate) => {
            // Validated by `promql::parse_query`: `rate()` never parses without one.
            let range_ms = parsed.range.expect("rate() always carries a range").as_millis() as i64;
            series_points
                .into_iter()
                .map(|(labels, points)| PromqlSeriesDto {
                    labels,
                    points: windowed_rate_series(&points, range_ms, start_ms, end_ms),
                })
                .collect()
        }
        Some(AggOp::Gauge) => {
            // Collapse each label combination's events down to their most-recent value in range — "value at time T", not a distinct stored gauge kind.
            series_points
                .into_iter()
                .filter_map(|(labels, points)| {
                    let last = *points.last()?;
                    Some(PromqlSeriesDto { labels, points: vec![last] })
                })
                .collect()
        }
        Some(AggOp::Histogram) => {
            // Validated by `promql::parse_query`: `histogram()` never parses
            // without one.
            let bucket_width =
                parsed.bucket_width.expect("histogram() always carries a bucket width");
            let last_ts = series_points
                .values()
                .flat_map(|points| points.iter().map(|(ts, _)| *ts))
                .max()
                .unwrap_or(0);
            // Groups series by `by_labels` first (empty `by_labels` collapses to a single group,
            // matching sum()/avg()'s behavior), then buckets each group's values independently into
            // fixed-width buckets, labeled `le` (Prometheus's less-than-or-equal convention) alongside
            // the group's own by_labels values.
            let mut groups: BTreeMap<BTreeMap<String, String>, Vec<&(i64, f64)>> = BTreeMap::new();
            for (labels, points) in &series_points {
                let group_key = project_labels(labels, &parsed.by_labels);
                groups.entry(group_key).or_default().extend(points.iter());
            }

            groups
                .into_iter()
                .flat_map(|(group_labels, points)| {
                    let mut counts: BTreeMap<i64, u64> = BTreeMap::new();
                    for (_, value) in points {
                        let bucket_index = (value / bucket_width).floor() as i64;
                        *counts.entry(bucket_index).or_insert(0) += 1;
                    }
                    counts.into_iter().map(move |(bucket_index, count)| {
                        let upper_bound = (bucket_index + 1) as f64 * bucket_width;
                        let mut labels = group_labels.clone();
                        labels.insert("le".to_string(), upper_bound.to_string());
                        PromqlSeriesDto { labels, points: vec![(last_ts, count as f64)] }
                    })
                })
                .collect()
        }
    })
}

#[cfg(test)]
mod tests {
    use super::*;

    fn request(name: &str, pattern: &str, capture_group: &str) -> DeclareParamRequest {
        DeclareParamRequest {
            name: name.to_string(),
            message_match_regex: pattern.to_string(),
            capture_group: capture_group.to_string(),
            json_field_path: None,
            kind: MetricPrimitiveKind::String,
            elapsed_since_last: false,
            auto_derived: false,
        }
    }

    #[test]
    fn declares_valid_param() {
        let mut index = BTreeMap::new();
        let result = declare_param(&mut index, request("level", r"level=(?P<level>\w+)", "level"));
        assert!(result.is_ok());

        let declarations: Vec<_> = param_declarations_range(&index).collect();
        assert_eq!(declarations.len(), 1);
        assert_eq!(declarations[0].0, "level");
        assert_eq!(declarations[0].1, r"level=(?P<level>\w+)");
        assert_eq!(declarations[0].2, "level");
    }

    #[test]
    fn auto_declare_params_discovers_new_keys_with_inferred_kinds_and_flags_them_auto() {
        let mut index = BTreeMap::new();
        auto_declare_params(&mut index, r#"ts=1788648105773 msg="entity_moved" x1=-4 y1=34"#);

        let declarations: HashMap<&str, _> =
            param_declarations_range(&index).map(|d| (d.0, d)).collect();
        assert_eq!(declarations.len(), 4);
        assert_eq!(declarations["x1"].4, MetricPrimitiveKind::Number);
        assert!(declarations["x1"].6, "expected x1 to be flagged auto_derived");
        assert_eq!(declarations["msg"].4, MetricPrimitiveKind::String);

        let compiled = compiled_param_declarations(&index);
        let params = derive_log_params(r#"ts=1788648105773 msg="entity_moved" x1=-4 y1=34"#, &compiled);
        assert_eq!(params.get("x1"), Some(&MetricPrimitive::Number(Number::Int(-4))));
        assert_eq!(params.get("msg"), Some(&MetricPrimitive::String("entity_moved".to_string())));
    }

    #[test]
    fn auto_declare_params_discovers_json_leaf_fields_via_json_field_path() {
        let mut index = BTreeMap::new();
        auto_declare_params(&mut index, r#"{"level":"error","code":500,"request":{"path":"/x"}}"#);

        let declarations: HashMap<&str, _> =
            param_declarations_range(&index).map(|d| (d.0, d)).collect();
        assert_eq!(declarations.len(), 3);
        assert_eq!(declarations["code"].4, MetricPrimitiveKind::Number);
        assert_eq!(declarations["code"].3, Some("code"));
        assert!(declarations["level"].6, "expected level to be flagged auto_derived");
        assert_eq!(declarations["request.path"].3, Some("request.path"));

        let compiled = compiled_param_declarations(&index);
        let params =
            derive_log_params(r#"{"level":"error","code":500,"request":{"path":"/x"}}"#, &compiled);
        assert_eq!(params.get("level"), Some(&MetricPrimitive::String("error".to_string())));
        assert_eq!(params.get("code"), Some(&MetricPrimitive::Number(Number::Int(500))));
        assert_eq!(params.get("request.path"), Some(&MetricPrimitive::String("/x".to_string())));
    }

    #[test]
    fn auto_declare_params_never_overwrites_an_existing_declaration() {
        let mut index = BTreeMap::new();
        declare_param(&mut index, request("x1", r#"x1="(?P<x1>[^"]+)""#, "x1")).unwrap();

        auto_declare_params(&mut index, "x1=-4");

        let declarations: Vec<_> = param_declarations_range(&index).collect();
        assert_eq!(declarations.len(), 1);
        assert_eq!(declarations[0].1, r#"x1="(?P<x1>[^"]+)""#, "manual declaration must survive untouched");
        assert!(!declarations[0].6, "a manual declaration must never be flagged auto_derived");
    }

    #[test]
    fn declare_param_rejects_editing_an_auto_derived_param() {
        let mut index = BTreeMap::new();
        auto_declare_params(&mut index, "x1=-4");

        let result = declare_param(&mut index, request("x1", r#"x1="(?P<x1>[^"]+)""#, "x1"));

        let err = result.unwrap_err();
        assert!(err.contains("auto-detected"), "unexpected error message: {err}");
        let declarations: Vec<_> = param_declarations_range(&index).collect();
        assert_eq!(declarations.len(), 1);
        assert!(declarations[0].6, "the original auto-derived declaration must be untouched");
    }

    const LOG_STREAM_FIXTURE: &str = include_str!("fixtures/log_stream_sample.txt");

    /// Runs both auto-declaration passes over every fixture line, same order as `EngineMessage::Ingest`.
    fn ingest_log_stream_fixture(index: &mut BTreeMap<IndexKey, DataValue>) {
        let mut header = None;
        for line in LOG_STREAM_FIXTURE.lines() {
            auto_declare_log_stream_params(index, &mut header, line);
            auto_declare_params(index, line);
        }
    }

    #[test]
    fn ingesting_log_stream_fixture_does_not_blow_up_declared_param_count() {
        // Ticket 050: unbounded auto-declaration on this real `log stream` dump produced 220 distinct
        // params. The bound (MAX_AUTO_DECLARED_PARAMS + the header-derived log_stream columns) must
        // keep the total in the tens, nowhere near "hundreds".
        let mut index = BTreeMap::new();
        ingest_log_stream_fixture(&mut index);

        let declared = param_declarations_range(&index).count();
        // The header-derived columns (ticket 059) are stored as `auto_derived` too, so the shared
        // `MAX_AUTO_DECLARED_PARAMS` ceiling covers all of them together (see
        // `auto_declare_log_stream_params`'s doc comment).
        assert!(
            declared <= MAX_AUTO_DECLARED_PARAMS,
            "expected at most {MAX_AUTO_DECLARED_PARAMS} declared params, got {declared}"
        );
        assert!(declared < 100, "declared param count ({declared}) should stay well under the old ~220");
    }

    #[test]
    fn base64_padding_fragments_in_log_stream_fixture_are_never_auto_declared() {
        let mut index = BTreeMap::new();
        ingest_log_stream_fixture(&mut index);

        let names: HashSet<&str> = param_declarations_range(&index).map(|d| d.0).collect();
        // The mDNSResponder lines' `<mask.hash: '...=='>` blobs would otherwise surface as params
        // named after the tail of the base64 payload (see `looks_like_base64_padding_artifact`).
        assert!(!names.contains("rKTEtG7JmnThhoKPbZiVrA"));
        assert!(!names.contains("gEdA8N4g"));
    }

    #[test]
    fn log_stream_shaped_lines_expose_header_named_columns_as_queryable_params() {
        let mut index = BTreeMap::new();
        ingest_log_stream_fixture(&mut index);

        let declared: HashSet<&str> = param_declarations_range(&index).map(|d| d.0).collect();
        // Derived generically from the fixture's own header row (ticket 059), not a fixed set of three.
        for name in ["Timestamp", "Thread", "Type", "Activity", "PID", "TTL", "message"] {
            assert!(declared.contains(name), "expected {name} to be declared, got {declared:?}");
        }

        let compiled = compiled_param_declarations(&index);
        let data_line = LOG_STREAM_FIXTURE.lines().nth(1).unwrap();
        let params = derive_log_params(data_line, &compiled);
        assert_eq!(params.get("Type"), Some(&MetricPrimitive::String("Default".to_string())));
        assert_eq!(params.get("PID"), Some(&MetricPrimitive::Number(Number::Int(1343))));
        assert_eq!(params.get("TTL"), Some(&MetricPrimitive::Number(Number::Int(7))));
        assert_eq!(
            params.get("message"),
            Some(&MetricPrimitive::String(
                "kitty: (SkyLight) [com.apple.SkyLight:default] SLSGetDisplaysWithOpenGLDisplayMask, 3614: Display 0x00000001: Mode 54, (0 x 0) [1728 x 1117], Depth 32 BPP, active".to_string()
            ))
        );
    }

    #[test]
    fn non_log_stream_lines_do_not_get_any_columns_declared_without_a_cached_header() {
        let mut index = BTreeMap::new();
        let mut header = None;
        auto_declare_log_stream_params(&mut index, &mut header, r#"ts=1788648105773 msg="entity_moved" x1=-4"#);

        assert!(param_declarations_range(&index).next().is_none());
        assert!(header.is_none());
    }

    #[test]
    #[ignore] // manual throughput measurement, not part of the regular suite
    fn bench_ingest_fixture_throughput() {
        let repeats = 50;
        let mut index = BTreeMap::new();
        let start = std::time::Instant::now();
        for _ in 0..repeats {
            let mut header = None;
            for line in LOG_STREAM_FIXTURE.lines() {
                auto_declare_log_stream_params(&mut index, &mut header, line);
                auto_declare_params(&mut index, line);
            }
        }
        let elapsed = start.elapsed();
        let total_lines = repeats * LOG_STREAM_FIXTURE.lines().count();
        eprintln!(
            "ingested {total_lines} lines in {elapsed:?} ({:.0} lines/sec), {} params declared",
            total_lines as f64 / elapsed.as_secs_f64(),
            param_declarations_range(&index).count(),
        );
    }

    #[test]
    fn rejects_invalid_regex() {
        let mut index = BTreeMap::new();
        let result = declare_param(&mut index, request("level", "level=(", "level"));
        assert!(result.is_err());
        assert!(index.is_empty());
    }

    #[test]
    fn rejects_unknown_capture_group() {
        let mut index = BTreeMap::new();
        let result = declare_param(
            &mut index,
            request("level", r"level=(?P<level>\w+)", "missing"),
        );
        assert!(result.is_err());
        assert!(index.is_empty());
    }

    #[test]
    fn declares_valid_elapsed_since_last_param_without_capture_group() {
        let mut index = BTreeMap::new();
        declare_elapsed_since_last_param(&mut index, "gap");

        let declarations: Vec<_> = param_declarations_range(&index).collect();
        assert_eq!(declarations.len(), 1);
        assert!(declarations[0].5, "expected elapsed_since_last to be true");
    }

    #[test]
    fn rejects_elapsed_since_last_param_with_non_duration_kind() {
        let mut index = BTreeMap::new();
        let result = declare_param(
            &mut index,
            DeclareParamRequest {
                name: "gap".to_string(),
                message_match_regex: ".".to_string(),
                capture_group: String::new(),
                json_field_path: None,
                kind: MetricPrimitiveKind::String,
                elapsed_since_last: true,
                auto_derived: false,
            },
        );
        assert!(result.is_err());
        assert!(index.is_empty());
    }

    #[test]
    fn declares_valid_param_from_json_field_path() {
        let mut index = BTreeMap::new();
        let result = declare_param(
            &mut index,
            DeclareParamRequest {
                name: "level".to_string(),
                // No named capture group needed for JSON-field extraction, just something that
                // matches the lines this param applies to.
                message_match_regex: r".".to_string(),
                capture_group: String::new(),
                json_field_path: Some("foo.bar".to_string()),
                kind: MetricPrimitiveKind::String,
                elapsed_since_last: false,
                auto_derived: false,
            },
        );
        assert!(result.is_ok());

        let declarations: Vec<_> = param_declarations_range(&index).collect();
        assert_eq!(declarations.len(), 1);
        assert_eq!(declarations[0].3, Some("foo.bar"));
    }

    #[test]
    fn derives_log_param_from_json_field_path() {
        let mut index = BTreeMap::new();
        declare_param(
            &mut index,
            DeclareParamRequest {
                name: "bar".to_string(),
                message_match_regex: r".".to_string(),
                capture_group: String::new(),
                json_field_path: Some("foo.bar".to_string()),
                kind: MetricPrimitiveKind::Number,
                elapsed_since_last: false,
                auto_derived: false,
            },
        )
        .unwrap();

        let declarations = compiled_param_declarations(&index);
        let params = derive_log_params(r#"{"foo":{"bar":42}}"#, &declarations);
        assert_eq!(params.get("bar"), Some(&MetricPrimitive::Number(Number::Int(42))));

        // A line that doesn't parse as JSON, or is missing the field, yields no param.
        assert!(derive_log_params("not json", &declarations).is_empty());
        assert!(derive_log_params(r#"{"foo":{}}"#, &declarations).is_empty());
    }

    #[test]
    fn evict_excess_logs_drops_oldest_sequences_first() {
        let mut index = BTreeMap::new();
        let mut log_count = 0;
        let mut approx_bytes_total = 0u64;
        for sequence in 0..5u64 {
            let text = format!("line {sequence}");
            approx_bytes_total += approx_log_bytes(&text);
            index.insert(
                IndexKey::IngestedLog { sequence, timestamp: Timestamp::from_millis(sequence as i64) },
                DataValue::LogLiteral { text },
            );
            log_count += 1;
        }

        evict_excess_logs(&mut index, &mut log_count, Some(3), &mut approx_bytes_total);

        assert_eq!(log_count, 3);
        let remaining: Vec<_> = logs_range(&index).map(|(seq, _, _)| seq).collect();
        assert_eq!(remaining, vec![2, 3, 4]);
        let expected_bytes: u64 =
            remaining.iter().map(|seq| approx_log_bytes(&format!("line {seq}"))).sum();
        assert_eq!(approx_bytes_total, expected_bytes);
    }

    #[test]
    fn evict_excess_logs_is_noop_when_unlimited_or_under_cap() {
        let mut index = BTreeMap::new();
        let mut log_count = 0;
        let mut approx_bytes_total = 0u64;
        for sequence in 0..3u64 {
            approx_bytes_total += approx_log_bytes("x");
            index.insert(
                IndexKey::IngestedLog { sequence, timestamp: Timestamp::from_millis(sequence as i64) },
                DataValue::LogLiteral { text: "x".to_string() },
            );
            log_count += 1;
        }

        evict_excess_logs(&mut index, &mut log_count, None, &mut approx_bytes_total);
        assert_eq!(log_count, 3);
        assert_eq!(approx_bytes_total, 3);

        evict_excess_logs(&mut index, &mut log_count, Some(10), &mut approx_bytes_total);
        assert_eq!(log_count, 3);
        assert_eq!(approx_bytes_total, 3);
    }

    fn insert_ingested_log(index: &mut BTreeMap<IndexKey, DataValue>, sequence: u64, text: &str) {
        index.insert(
            IndexKey::IngestedLog { sequence, timestamp: Timestamp::from_millis(sequence as i64) },
            DataValue::LogLiteral { text: text.to_string() },
        );
    }

    #[test]
    fn clear_all_param_declarations_removes_manual_and_auto_derived() {
        let mut index = BTreeMap::new();
        auto_declare_params(&mut index, "x1=-4");
        declare_param(&mut index, request("level", r"level=(?P<level>\w+)", "level")).unwrap();
        assert_eq!(param_declarations_range(&index).count(), 2);

        let removed = clear_all_param_declarations(&mut index);

        assert_eq!(removed, 2);
        assert_eq!(param_declarations_range(&index).count(), 0);
    }

    #[test]
    fn reextract_params_from_retained_logs_rebuilds_from_ingested_text_without_new_ingest() {
        let mut index = BTreeMap::new();
        insert_ingested_log(&mut index, 0, r#"level=info msg="started""#);
        insert_ingested_log(&mut index, 1, "x1=-4 y1=34");
        auto_declare_params(&mut index, r#"level=info msg="started""#);
        auto_declare_params(&mut index, "x1=-4 y1=34");
        assert_eq!(param_declarations_range(&index).count(), 4);

        clear_all_param_declarations(&mut index);
        assert_eq!(param_declarations_range(&index).count(), 0);

        // No new lines ingested — re-extraction works purely off the `IngestedLog` entries already
        // retained in the index (ticket 052's core acceptance criterion).
        let (reextracted, logs_considered) = reextract_params_from_retained_logs(&mut index);

        assert_eq!(logs_considered, 2);
        assert_eq!(reextracted, 4);
        let declarations: HashMap<&str, _> =
            param_declarations_range(&index).map(|d| (d.0, d)).collect();
        assert!(declarations.contains_key("level"));
        assert!(declarations.contains_key("msg"));
        assert!(declarations.contains_key("x1"));
        assert!(declarations.contains_key("y1"));
        assert!(declarations.values().all(|d| d.6), "re-extracted params must be flagged auto_derived");
    }

    #[test]
    fn reextract_params_from_retained_logs_only_covers_lines_still_in_the_index() {
        // Simulates a line evicted (by `max_logs`) before the clear+re-extract runs: its key/value
        // pair is simply gone from the index, same as `evict_excess_logs` leaves things, so
        // `auto_declare_params` never sees it again.
        let mut index = BTreeMap::new();
        insert_ingested_log(&mut index, 1, "x1=-4");
        auto_declare_params(&mut index, "evicted_only_key=1");
        auto_declare_params(&mut index, "x1=-4");
        assert_eq!(param_declarations_range(&index).count(), 2);

        clear_all_param_declarations(&mut index);
        let (reextracted, logs_considered) = reextract_params_from_retained_logs(&mut index);

        assert_eq!(logs_considered, 1, "only the retained log line should be re-scanned");
        assert_eq!(reextracted, 1, "the evicted line's key can't come back");
        let declarations: Vec<_> = param_declarations_range(&index).collect();
        assert_eq!(declarations[0].0, "x1");
    }

    #[test]
    fn declarations_stay_within_their_range() {
        let mut index = BTreeMap::new();
        index.insert(
            IndexKey::IngestedLog { sequence: 0, timestamp: Timestamp::from_millis(0) },
            DataValue::LogLiteral { text: "hello".to_string() },
        );
        declare_param(&mut index, request("level", r"level=(?P<level>\w+)", "level")).unwrap();
        index.insert(
            IndexKey::MetricDeclaration { name: "requests".to_string() },
            DataValue::MetricDeclaration {
                message_match_regex: "x".to_string(),
                label_params: vec![],
                value_param: None,
                matched_labels: None,
                unmatched_labels: None,
            },
        );

        let declarations: Vec<_> = param_declarations_range(&index).collect();
        assert_eq!(declarations.len(), 1);
        assert_eq!(declarations[0].0, "level");

        for key in index.range(IndexKey::LogParamSplit..IndexKey::ParamMetricSplit).map(|(k, _)| k) {
            assert!(matches!(key, IndexKey::ParamDeclaration { .. }));
        }
    }

    #[test]
    fn removes_declaration() {
        let mut index = BTreeMap::new();
        declare_param(&mut index, request("level", r"level=(?P<level>\w+)", "level")).unwrap();
        assert!(index.remove(&IndexKey::ParamDeclaration { name: "level".to_string() }).is_some());
        assert!(param_declarations_range(&index).next().is_none());
    }

    /// Declares a standalone param whose extractor points at `pattern`'s named `capture_group` — the
    /// shape most of these tests need so a metric can then reference it by name via `label_params`/
    /// `value_param` instead of embedding its own extractor (ticket 037).
    fn declare_test_param(
        index: &mut BTreeMap<IndexKey, DataValue>,
        name: &str,
        pattern: &str,
        capture_group: &str,
        kind: MetricPrimitiveKind,
    ) {
        declare_param(
            index,
            DeclareParamRequest {
                name: name.to_string(),
                message_match_regex: pattern.to_string(),
                capture_group: capture_group.to_string(),
                json_field_path: None,
                kind,
                elapsed_since_last: false,
                auto_derived: false,
            },
        )
        .unwrap();
    }

    /// Declares a standalone `elapsed_since_last` param (ticket 038) — computed as elapsed time
    /// since the previous event sharing whatever label set it's grouped under, never text-extracted.
    /// `message_match_regex`/`capture_group` are unused for this extractor but still need to satisfy
    /// `declare_param`'s baseline "is this a valid regex" check, so an always-matching `"."` is fine.
    fn declare_elapsed_since_last_param(index: &mut BTreeMap<IndexKey, DataValue>, name: &str) {
        declare_param(
            index,
            DeclareParamRequest {
                name: name.to_string(),
                message_match_regex: ".".to_string(),
                capture_group: String::new(),
                json_field_path: None,
                kind: MetricPrimitiveKind::Duration,
                elapsed_since_last: true,
                auto_derived: false,
            },
        )
        .unwrap();
    }

    fn metric_request(name: &str, pattern: &str, label_params: Vec<&str>) -> DeclareMetricRequest {
        metric_request_with_value(name, pattern, label_params, None)
    }

    fn metric_request_with_value(
        name: &str,
        pattern: &str,
        label_params: Vec<&str>,
        value_param: Option<&str>,
    ) -> DeclareMetricRequest {
        DeclareMetricRequest {
            name: name.to_string(),
            message_match_regex: pattern.to_string(),
            label_params: label_params.into_iter().map(str::to_string).collect(),
            value_param: value_param.map(str::to_string),
            matched_labels: None,
            unmatched_labels: None,
        }
    }

    #[test]
    fn declares_valid_metric() {
        let mut index = BTreeMap::new();
        let pattern = r"request path=(?P<path>\S+) duration=(?P<duration>\d+)";
        declare_test_param(&mut index, "path", pattern, "path", MetricPrimitiveKind::String);
        declare_test_param(&mut index, "duration", pattern, "duration", MetricPrimitiveKind::Number);

        let result = declare_metric(&mut index, metric_request("requests", pattern, vec!["path", "duration"]));
        assert!(result.is_ok());

        let declarations: Vec<_> = metric_declarations_range(&index).collect();
        assert_eq!(declarations.len(), 1);
        assert_eq!(declarations[0].0, "requests");
        assert_eq!(declarations[0].1, pattern);
        assert_eq!(declarations[0].2.len(), 2);
    }

    #[test]
    fn rejects_invalid_metric_regex() {
        let mut index = BTreeMap::new();
        let result = declare_metric(&mut index, metric_request("requests", "path=(", vec![]));
        assert!(result.is_err());
        assert!(index.is_empty());
    }

    #[test]
    fn rejects_metric_referencing_undeclared_param() {
        let mut index = BTreeMap::new();
        let pattern = r"request path=(?P<path>\S+)";
        declare_test_param(&mut index, "path", pattern, "path", MetricPrimitiveKind::String);

        // "duration" is never declared as a standalone param.
        let result = declare_metric(&mut index, metric_request("requests", pattern, vec!["path", "duration"]));
        assert!(result.is_err());
        assert!(result.unwrap_err().contains("duration"));
        assert!(!index.contains_key(&IndexKey::MetricDeclaration { name: "requests".to_string() }));
    }

    #[test]
    fn metric_declarations_stay_within_their_range() {
        let mut index = BTreeMap::new();
        index.insert(
            IndexKey::IngestedLog { sequence: 0, timestamp: Timestamp::from_millis(0) },
            DataValue::LogLiteral { text: "hello".to_string() },
        );
        declare_param(&mut index, request("level", r"level=(?P<level>\w+)", "level")).unwrap();
        let pattern = r"duration=(?P<duration>\d+)";
        declare_test_param(&mut index, "duration", pattern, "duration", MetricPrimitiveKind::Number);
        declare_metric(&mut index, metric_request("requests", pattern, vec!["duration"])).unwrap();

        let declarations: Vec<_> = metric_declarations_range(&index).collect();
        assert_eq!(declarations.len(), 1);
        assert_eq!(declarations[0].0, "requests");

        for key in index.range(IndexKey::ParamMetricSplit..IndexKey::MetricBucketSplit).map(|(k, _)| k) {
            assert!(matches!(key, IndexKey::MetricDeclaration { .. }));
        }
    }

    #[test]
    fn removes_metric_declaration() {
        let mut index = BTreeMap::new();
        let pattern = r"duration=(?P<duration>\d+)";
        declare_test_param(&mut index, "duration", pattern, "duration", MetricPrimitiveKind::Number);
        declare_metric(&mut index, metric_request("requests", pattern, vec!["duration"])).unwrap();
        assert!(index.remove(&IndexKey::MetricDeclaration { name: "requests".to_string() }).is_some());
        assert!(metric_declarations_range(&index).next().is_none());
    }

    #[test]
    fn remove_metric_declaration_reports_whether_it_existed() {
        let mut index = BTreeMap::new();
        declare_requests_metric(&mut index);
        assert!(remove_metric_declaration(&mut index, "requests"));
        // Removing an already-gone (or never-declared) metric is a clean `false`, not a panic —
        // this is what the API layer's 404 branch relies on.
        assert!(!remove_metric_declaration(&mut index, "requests"));
        assert!(!remove_metric_declaration(&mut index, "never-declared"));
    }

    /// Ticket 051: deleting a metric purges its bucket history too, so a stale PromQL query
    /// (e.g. a dashboard panel that still names the deleted metric) fails with a readable
    /// "unknown metric" error instead of silently returning orphaned data.
    #[test]
    fn remove_metric_declaration_purges_bucket_history_so_promql_reports_unknown_metric() {
        let mut index = BTreeMap::new();
        declare_requests_metric(&mut index);
        ingest_log(&mut index, 0, "request path=/a");

        let parsed = promql::parse_query("requests").unwrap();
        assert!(evaluate_promql(&index, &parsed, None, None).unwrap().len() == 1);

        assert!(remove_metric_declaration(&mut index, "requests"));
        assert!(bucket_values(&index, "requests", None).next().is_none());

        let err = evaluate_promql(&index, &parsed, None, None).unwrap_err();
        assert!(err.contains("unknown metric"), "unexpected error message: {err}");
    }

    /// A metric that was ingested via `--metrics-url` scraping (`IngestMetricSample`) never gets a
    /// `MetricDeclaration` at all, so the "unknown metric" check must key off having *either* a
    /// declaration *or* recorded data — not declaration alone — or every scraped metric would be
    /// rejected as unknown.
    #[test]
    fn undeclared_metric_with_bucket_data_is_still_queryable_via_promql() {
        let mut index = BTreeMap::new();
        push_bucket_value(
            &mut index,
            "scraped_metric",
            BTreeMap::new(),
            0,
            Timestamp::from_millis(0),
            MetricPrimitive::Number(Number::Float(1.0)),
        );
        let parsed = promql::parse_query("scraped_metric").unwrap();
        let series = evaluate_promql(&index, &parsed, None, None).unwrap();
        assert_eq!(series.len(), 1);
    }

    #[test]
    fn promql_rejects_metric_with_no_declaration_and_no_data() {
        let index = BTreeMap::new();
        let parsed = promql::parse_query("never_declared").unwrap();
        let err = evaluate_promql(&index, &parsed, None, None).unwrap_err();
        assert!(err.contains("unknown metric"), "unexpected error message: {err}");
    }

    #[test]
    fn update_metric_rejects_when_no_declaration_exists_yet() {
        let mut index = BTreeMap::new();
        let result = update_metric(
            &mut index,
            "requests",
            metric_request("requests", r"request path=(?P<path>\S+)", vec![]),
        );
        assert!(result.is_err());
        assert!(result.unwrap_err().contains("requests"));
    }

    #[test]
    fn update_metric_reuses_declare_metric_validation() {
        let mut index = BTreeMap::new();
        declare_metric(&mut index, metric_request("requests", r"request path=(?P<path>\S+)", vec![])).unwrap();

        // "duration" is never declared as a standalone param.
        let result = update_metric(
            &mut index,
            "requests",
            metric_request("requests", r"request path=(?P<path>\S+)", vec!["duration"]),
        );
        assert!(result.is_err());
        assert!(result.unwrap_err().contains("duration"));
    }

    #[test]
    fn update_metric_replaces_declaration_in_place_without_duplicating() {
        let mut index = BTreeMap::new();
        let pattern = r"request path=(?P<path>\S+) duration=(?P<duration>\d+)";
        declare_test_param(&mut index, "path", pattern, "path", MetricPrimitiveKind::String);
        declare_metric(&mut index, metric_request("requests", r"request path=(?P<path>\S+)", vec!["path"])).unwrap();

        declare_test_param(&mut index, "duration", pattern, "duration", MetricPrimitiveKind::Number);
        let result = update_metric(&mut index, "requests", metric_request("requests", pattern, vec!["path", "duration"]));
        assert!(result.is_ok());

        // Still one declaration named "requests", now with the updated regex/params.
        let declarations: Vec<_> = metric_declarations_range(&index).collect();
        assert_eq!(declarations.len(), 1);
        assert_eq!(declarations[0].0, "requests");
        assert_eq!(declarations[0].2.len(), 2);
    }

    #[test]
    fn update_metric_rebackfills_bucket_data_from_the_new_declaration() {
        let mut index = BTreeMap::new();
        ingest_log(&mut index, 0, "request path=/a");
        ingest_log(&mut index, 1, "request path=/b");
        declare_test_param(&mut index, "path", r"request path=(?P<path>\S+)", "path", MetricPrimitiveKind::String);
        declare_metric(&mut index, metric_request("requests", r"request path=(?P<path>\S+)", vec!["path"])).unwrap();
        assert_eq!(bucket_values(&index, "requests", None).count(), 2);

        // Editing to a narrower regex should leave exactly the buckets matching the *new* declaration — not the old ones layered underneath; the "path" param's own regex is unaffected, so it still extracts fine.
        update_metric(&mut index, "requests", metric_request("requests", r"request path=(?P<path>/a)", vec!["path"])).unwrap();
        assert_eq!(bucket_values(&index, "requests", None).count(), 1);
    }

    #[test]
    fn update_metric_can_rename_and_rejects_a_rename_onto_an_existing_name() {
        let mut index = BTreeMap::new();
        declare_metric(&mut index, metric_request("requests", r"request path=(?P<path>\S+)", vec![])).unwrap();
        declare_metric(&mut index, metric_request("errors", r"error code=(?P<code>\d+)", vec![])).unwrap();

        // Renaming onto an already-taken name is rejected.
        let result = update_metric(
            &mut index,
            "requests",
            metric_request("errors", r"request path=(?P<path>\S+)", vec![]),
        );
        assert!(result.is_err());

        // Renaming onto a fresh name succeeds and drops the old declaration.
        update_metric(
            &mut index,
            "requests",
            metric_request("http_requests", r"request path=(?P<path>\S+)", vec![]),
        )
        .unwrap();
        let names: Vec<_> = metric_declarations_range(&index).map(|(name, ..)| name.to_string()).collect();
        assert!(names.contains(&"http_requests".to_string()));
        assert!(!names.contains(&"requests".to_string()));
    }

    fn ingest_log(index: &mut BTreeMap<IndexKey, DataValue>, sequence: u64, text: &str) {
        let timestamp = Timestamp::from_millis(sequence as i64);
        index.insert(
            IndexKey::IngestedLog { sequence, timestamp },
            DataValue::LogLiteral { text: text.to_string() },
        );
        apply_all_metrics(index, sequence, timestamp, text);
    }

    #[test]
    fn declaring_metric_backfills_matching_logs() {
        let mut index = BTreeMap::new();
        ingest_log(&mut index, 0, "request path=/a duration=1");
        ingest_log(&mut index, 1, "request path=/b duration=2");
        ingest_log(&mut index, 2, "unrelated line");

        let pattern = r"request path=(?P<path>\S+) duration=(?P<duration>\d+)";
        declare_test_param(&mut index, "path", pattern, "path", MetricPrimitiveKind::String);
        declare_metric(&mut index, metric_request("requests", pattern, vec!["path"])).unwrap();

        let values: Vec<_> = bucket_values(&index, "requests", None).collect();
        assert_eq!(values.len(), 2);
    }

    #[test]
    fn metric_param_extracts_from_json_field_path() {
        let mut index = BTreeMap::new();
        ingest_log(&mut index, 0, r#"{"request":{"path":"/a"}}"#);
        // Doesn't parse as JSON at all — should be skipped, not panic.
        ingest_log(&mut index, 1, "not json");
        // Parses as JSON but is missing the field — also skipped.
        ingest_log(&mut index, 2, r#"{"request":{}}"#);

        declare_param(
            &mut index,
            DeclareParamRequest {
                name: "path".to_string(),
                message_match_regex: r".".to_string(),
                capture_group: String::new(),
                json_field_path: Some("request.path".to_string()),
                kind: MetricPrimitiveKind::String,
                elapsed_since_last: false,
                auto_derived: false,
            },
        )
        .unwrap();
        declare_metric(&mut index, metric_request("requests", r".", vec!["path"])).unwrap();

        let values: Vec<_> = bucket_values(&index, "requests", None).collect();
        assert_eq!(values.len(), 1);
        assert_eq!(
            values[0].0.get("path"),
            Some(&MetricPrimitive::String("/a".to_string()))
        );
    }

    #[test]
    fn ingesting_after_declaration_updates_buckets_live() {
        let mut index = BTreeMap::new();
        declare_test_param(&mut index, "path", r"request path=(?P<path>\S+)", "path", MetricPrimitiveKind::String);
        declare_metric(&mut index, metric_request("requests", r"request path=(?P<path>\S+)", vec!["path"])).unwrap();

        ingest_log(&mut index, 0, "request path=/a");

        let values: Vec<_> = bucket_values(&index, "requests", None).collect();
        assert_eq!(values.len(), 1);
        assert_eq!(values[0].1.0, 0);
    }

    #[test]
    fn non_matching_log_leaves_bucket_range_untouched() {
        let mut index = BTreeMap::new();
        declare_test_param(&mut index, "path", r"request path=(?P<path>\S+)", "path", MetricPrimitiveKind::String);
        declare_metric(&mut index, metric_request("requests", r"request path=(?P<path>\S+)", vec!["path"])).unwrap();

        ingest_log(&mut index, 0, "totally unrelated line");

        assert!(
            index
                .range(IndexKey::MetricBucketSplit..IndexKey::End)
                .find(|(key, _)| matches!(key, IndexKey::Bucket { .. }))
                .is_none()
        );
    }

    #[test]
    fn bucket_values_preserve_insertion_order() {
        let mut index = BTreeMap::new();
        declare_test_param(&mut index, "path", r"request path=(?P<path>\S+)", "path", MetricPrimitiveKind::String);
        declare_metric(&mut index, metric_request("requests", r"request path=(?P<path>\S+)", vec!["path"])).unwrap();

        ingest_log(&mut index, 5, "request path=/a");
        ingest_log(&mut index, 2, "request path=/a");
        ingest_log(&mut index, 9, "request path=/a");

        let sequences: Vec<u64> = bucket_values(&index, "requests", None)
            .map(|(_, (sequence, _, _))| *sequence)
            .collect();
        assert_eq!(sequences, vec![5, 2, 9]);
    }

    #[test]
    fn differing_label_combinations_produce_distinct_buckets() {
        let mut index = BTreeMap::new();
        declare_test_param(&mut index, "path", r"request path=(?P<path>\S+)", "path", MetricPrimitiveKind::String);
        declare_metric(&mut index, metric_request("requests", r"request path=(?P<path>\S+)", vec!["path"])).unwrap();

        ingest_log(&mut index, 0, "request path=/a");
        ingest_log(&mut index, 1, "request path=/b");

        let bucket_keys: Vec<_> = index
            .range(IndexKey::MetricBucketSplit..IndexKey::End)
            .filter(|(key, _)| matches!(key, IndexKey::Bucket { .. }))
            .collect();
        assert_eq!(bucket_keys.len(), 2);
        for (_, value) in &bucket_keys {
            match value {
                DataValue::BucketValue { values } => assert_eq!(values.len(), 1),
                _ => panic!("expected BucketValue"),
            }
        }
    }

    #[test]
    fn single_log_line_can_match_multiple_metrics() {
        let mut index = BTreeMap::new();
        let pattern = r"request path=(?P<path>\S+) duration=(?P<duration>\d+)";
        declare_test_param(&mut index, "path", pattern, "path", MetricPrimitiveKind::String);
        declare_test_param(&mut index, "duration", pattern, "duration", MetricPrimitiveKind::Number);
        declare_metric(&mut index, metric_request("requests", pattern, vec!["path"])).unwrap();
        declare_metric(&mut index, metric_request("durations", pattern, vec!["duration"])).unwrap();

        ingest_log(&mut index, 0, "request path=/a duration=5");

        assert_eq!(bucket_values(&index, "requests", None).count(), 1);
        assert_eq!(bucket_values(&index, "durations", None).count(), 1);
    }

    fn declare_requests_metric(index: &mut BTreeMap<IndexKey, DataValue>) {
        declare_test_param(index, "path", r"request path=(?P<path>\S+)", "path", MetricPrimitiveKind::String);
        declare_metric(index, metric_request("requests", r"request path=(?P<path>\S+)", vec!["path"])).unwrap();
    }

    #[test]
    fn promql_query_selects_matching_label_combination() {
        let mut index = BTreeMap::new();
        declare_requests_metric(&mut index);
        ingest_log(&mut index, 0, "request path=/a");
        ingest_log(&mut index, 1, "request path=/b");
        ingest_log(&mut index, 2, "request path=/a");

        let parsed = promql::parse_query(r#"requests{path="/a"}"#).unwrap();
        let series = evaluate_promql(&index, &parsed, None, None).unwrap();

        assert_eq!(series.len(), 1);
        assert_eq!(series[0].labels.get("path").map(String::as_str), Some("/a"));
        // Occurrence markers coerce to 1.0 (see coerce_to_f64); timestamps come from ingest_log's sequence-as-millis helper.
        assert_eq!(series[0].points, vec![(0, 1.0), (2, 1.0)]);
    }

    #[test]
    fn promql_query_with_no_matching_labels_returns_no_series() {
        let mut index = BTreeMap::new();
        declare_requests_metric(&mut index);
        ingest_log(&mut index, 0, "request path=/a");

        let parsed = promql::parse_query(r#"requests{path="/nope"}"#).unwrap();
        let series = evaluate_promql(&index, &parsed, None, None).unwrap();
        assert!(series.is_empty());
    }

    #[test]
    fn promql_sum_aggregates_across_label_combinations() {
        let mut index = BTreeMap::new();
        declare_requests_metric(&mut index);
        ingest_log(&mut index, 0, "request path=/a");
        ingest_log(&mut index, 0, "request path=/b");

        let parsed = promql::parse_query("sum(requests)").unwrap();
        let series = evaluate_promql(&index, &parsed, None, None).unwrap();

        assert_eq!(series.len(), 1);
        assert!(series[0].labels.is_empty());
        assert_eq!(series[0].points, vec![(0, 2.0)]);
    }

    #[test]
    fn promql_sum_by_preserves_the_named_label_dimension() {
        let mut index = BTreeMap::new();
        declare_requests_metric(&mut index);
        ingest_log(&mut index, 0, "request path=/a");
        ingest_log(&mut index, 0, "request path=/b");
        ingest_log(&mut index, 1, "request path=/a");

        let parsed = promql::parse_query("sum(requests) by (path)").unwrap();
        let mut series = evaluate_promql(&index, &parsed, None, None).unwrap();
        series.sort_by(|a, b| a.labels.get("path").cmp(&b.labels.get("path")));

        assert_eq!(series.len(), 2);
        assert_eq!(series[0].labels.get("path").map(String::as_str), Some("/a"));
        assert_eq!(series[0].points, vec![(0, 1.0), (1, 1.0)]);
        assert_eq!(series[1].labels.get("path").map(String::as_str), Some("/b"));
        assert_eq!(series[1].points, vec![(0, 1.0)]);
    }

    /// Declares a metric whose captured `status` param is `String`-kind and holds the `Value` role,
    /// so every bucket entry for it is a `MetricPrimitive::String` rather than the default occurrence marker.
    fn declare_string_valued_metric(index: &mut BTreeMap<IndexKey, DataValue>) {
        declare_test_param(
            index,
            "status",
            r"request status=(?P<status>\S+)",
            "status",
            MetricPrimitiveKind::String,
        );
        declare_metric(
            index,
            metric_request_with_value(
                "statuses",
                r"request status=(?P<status>\S+)",
                vec![],
                Some("status"),
            ),
        )
        .unwrap();
    }

    fn declare_requests_by_host_metric(index: &mut BTreeMap<IndexKey, DataValue>) {
        declare_test_param(
            index,
            "path",
            r"request path=(?P<path>\S+) host=(?P<host>\S+)",
            "path",
            MetricPrimitiveKind::String,
        );
        declare_test_param(
            index,
            "host",
            r"request path=(?P<path>\S+) host=(?P<host>\S+)",
            "host",
            MetricPrimitiveKind::String,
        );
        declare_metric(
            index,
            metric_request(
                "requests",
                r"request path=(?P<path>\S+) host=(?P<host>\S+)",
                vec!["path", "host"],
            ),
        )
        .unwrap();
    }

    #[test]
    fn sum_over_string_kind_metric_is_rejected_with_an_actionable_error() {
        let mut index = BTreeMap::new();
        declare_string_valued_metric(&mut index);
        ingest_log(&mut index, 0, "request status=ok");

        let parsed = promql::parse_query("sum(statuses)").unwrap();
        let err = evaluate_promql(&index, &parsed, None, None).unwrap_err();

        assert!(err.contains("statuses"), "error should name the offending metric: {err}");
        assert!(err.to_lowercase().contains("string"), "error should explain why: {err}");
    }

    #[test]
    fn avg_over_string_kind_metric_is_rejected_with_an_actionable_error() {
        let mut index = BTreeMap::new();
        declare_string_valued_metric(&mut index);
        ingest_log(&mut index, 0, "request status=ok");

        let parsed = promql::parse_query("avg(statuses)").unwrap();
        let err = evaluate_promql(&index, &parsed, None, None).unwrap_err();

        assert!(err.contains("statuses"));
    }

    #[test]
    fn count_over_string_kind_metric_counts_occurrences_by_name_not_by_accident() {
        let mut index = BTreeMap::new();
        declare_string_valued_metric(&mut index);
        ingest_log(&mut index, 0, "request status=ok");
        ingest_log(&mut index, 1, "request status=error");

        let parsed = promql::parse_query("count(statuses)").unwrap();
        let series = evaluate_promql(&index, &parsed, None, None).unwrap();

        assert_eq!(series.len(), 1);
        assert!(series[0].labels.is_empty());
        assert_eq!(series[0].points, vec![(0, 1.0), (1, 1.0)]);
    }

    #[test]
    fn sum_over_numeric_kind_metric_is_unaffected_by_the_string_kind_guard() {
        let mut index = BTreeMap::new();
        declare_requests_metric(&mut index);
        ingest_log(&mut index, 0, "request path=/a");
        ingest_log(&mut index, 0, "request path=/b");

        let parsed = promql::parse_query("sum(requests)").unwrap();
        let series = evaluate_promql(&index, &parsed, None, None).unwrap();

        assert_eq!(series.len(), 1);
        assert_eq!(series[0].points, vec![(0, 2.0)]);
    }

    #[test]
    fn sum_of_rate_over_string_kind_metric_is_still_allowed_since_rate_only_counts_occurrences() {
        let mut index = BTreeMap::new();
        declare_string_valued_metric(&mut index);
        ingest_log(&mut index, 0, "request status=ok");
        ingest_log(&mut index, 1000, "request status=error");

        let parsed = promql::parse_query("sum(rate(statuses[2s]))").unwrap();
        let series = evaluate_promql(&index, &parsed, None, None);

        assert!(series.is_ok(), "sum(rate(...)) never reads the coerced value, so it should not be rejected: {series:?}");
    }

    #[test]
    fn bare_query_over_string_kind_metric_is_still_allowed() {
        let mut index = BTreeMap::new();
        declare_string_valued_metric(&mut index);
        ingest_log(&mut index, 0, "request status=ok");

        let parsed = promql::parse_query("statuses").unwrap();
        let series = evaluate_promql(&index, &parsed, None, None);

        assert!(series.is_ok());
    }

    #[test]
    fn sum_by_merges_staggered_raw_timestamps_across_label_groups_instead_of_going_sparse() {
        // Regression test for ticket 042: `sum(requests) by (path)` groups together several
        // underlying (path, host) series. Their raw occurrence timestamps are staggered (as real
        // occurrence timestamps essentially always are), so a merge that only combines exact-
        // timestamp matches (the old `merge_series_exact` behavior) would leave each point
        // reflecting just one host instead of the combined total across hosts.
        let mut index = BTreeMap::new();
        declare_requests_by_host_metric(&mut index);
        ingest_log(&mut index, 0, "request path=/a host=web1");
        ingest_log(&mut index, 1, "request path=/a host=web2");
        ingest_log(&mut index, 2, "request path=/a host=web3");

        let parsed = promql::parse_query("sum(requests) by (path)").unwrap();
        let series = evaluate_promql(&index, &parsed, None, None).unwrap();

        assert_eq!(series.len(), 1);
        assert_eq!(series[0].labels.get("path").map(String::as_str), Some("/a"));
        // The three staggered per-host occurrences merge into one combined-total point rather than
        // three separate points each showing a lone host's count of 1.
        let total: f64 = series[0].points.iter().map(|(_, v)| v).sum();
        assert_eq!(total, 3.0, "expected all three staggered occurrences to sum to 3.0, got {:?}", series[0].points);
        assert!(
            series[0].points.len() < 3,
            "expected staggered timestamps to be bucketed together rather than staying sparse/near-empty, got {:?}",
            series[0].points
        );
    }

    #[test]
    fn avg_by_merges_staggered_raw_timestamps_across_label_groups() {
        let mut index = BTreeMap::new();
        declare_requests_by_host_metric(&mut index);
        ingest_log(&mut index, 0, "request path=/a host=web1");
        ingest_log(&mut index, 1, "request path=/a host=web2");

        let parsed = promql::parse_query("avg(requests) by (path)").unwrap();
        let series = evaluate_promql(&index, &parsed, None, None).unwrap();

        assert_eq!(series.len(), 1);
        assert_eq!(series[0].labels.get("path").map(String::as_str), Some("/a"));
        // Both occurrences (value 1.0 each) land in the same merged bucket, so the average stays 1.0
        // rather than reporting two separate points that each only saw one host.
        assert_eq!(series[0].points, vec![(0, 1.0)]);
    }

    #[test]
    fn sum_by_still_gives_a_single_label_group_its_own_line_at_full_resolution() {
        // Existing single-label-group behavior (only one underlying series feeds each output group)
        // must stay untouched by the multi-series windowing fix.
        let mut index = BTreeMap::new();
        declare_requests_metric(&mut index);
        ingest_log(&mut index, 0, "request path=/a");
        ingest_log(&mut index, 1, "request path=/a");

        let parsed = promql::parse_query("sum(requests) by (path)").unwrap();
        let series = evaluate_promql(&index, &parsed, None, None).unwrap();

        assert_eq!(series.len(), 1);
        assert_eq!(series[0].points, vec![(0, 1.0), (1, 1.0)]);
    }

    #[test]
    fn bare_rate_produces_a_windowed_point_per_timestamp_not_a_single_point() {
        let mut index = BTreeMap::new();
        declare_requests_metric(&mut index);
        ingest_log(&mut index, 0, "request path=/a");
        ingest_log(&mut index, 1000, "request path=/a");

        let parsed = promql::parse_query("rate(requests[2s])").unwrap();
        let series = evaluate_promql(&index, &parsed, None, None).unwrap();

        assert_eq!(series.len(), 1);
        assert_eq!(series[0].points, vec![(0, 0.5), (1000, 1.0)]);
    }

    #[test]
    fn sum_of_rate_by_label_gives_each_label_value_its_own_line() {
        let mut index = BTreeMap::new();
        declare_requests_metric(&mut index);
        ingest_log(&mut index, 0, "request path=/a");
        ingest_log(&mut index, 1000, "request path=/a");
        ingest_log(&mut index, 0, "request path=/b");

        let parsed = promql::parse_query("sum(rate(requests[2s])) by (path)").unwrap();
        let mut series = evaluate_promql(&index, &parsed, None, None).unwrap();
        series.sort_by(|a, b| a.labels.get("path").cmp(&b.labels.get("path")));

        assert_eq!(series.len(), 2);
        assert_eq!(series[0].labels.get("path").map(String::as_str), Some("/a"));
        assert_eq!(series[0].points, vec![(0, 0.5), (1000, 1.0)]);
        assert_eq!(series[1].labels.get("path").map(String::as_str), Some("/b"));
        assert_eq!(series[1].points, vec![(0, 0.5)]);
    }

    /// Ticket 057: reproduces the reported `sum by (code) (rate(KeyPressed[5m]))` chart — a burst
    /// of occurrences confined to the very start of a much wider queried range. Without a
    /// synthetic trailing point, the series would just stop at the last occurrence's timestamp
    /// (`1`), giving no indication the rate has decayed to 0 for the rest of `end_ms`.
    #[test]
    fn rate_series_gets_a_trailing_zero_point_once_occurrences_stop_early_in_a_wider_range() {
        let mut index = BTreeMap::new();
        declare_requests_metric(&mut index);
        ingest_log(&mut index, 0, "request path=/a");
        ingest_log(&mut index, 1, "request path=/a");

        let parsed = promql::parse_query("rate(requests[2s])").unwrap();
        let series = evaluate_promql(&index, &parsed, None, Some(100_000)).unwrap();

        assert_eq!(series.len(), 1);
        assert_eq!(
            series[0].points,
            vec![(0, 0.5), (1, 1.0), (100_000, 0.0)],
            "the window (2s) is long since aged out by end_ms, so the appended boundary point must read 0"
        );
    }

    /// Mirror of the trailing case: occurrences starting well after `start_ms` shouldn't make the
    /// line appear to begin mid-range with no indication the rate was 0 beforehand.
    #[test]
    fn rate_series_gets_a_leading_zero_point_when_occurrences_start_late_in_a_wider_range() {
        let mut index = BTreeMap::new();
        declare_requests_metric(&mut index);
        ingest_log(&mut index, 50_000, "request path=/a");

        let parsed = promql::parse_query("rate(requests[2s])").unwrap();
        let series = evaluate_promql(&index, &parsed, Some(0), None).unwrap();

        assert_eq!(series[0].points, vec![(0, 0.0), (50_000, 0.5)]);
    }

    /// Same shape as the two tests above, but through `sum(rate(...)) by (label)` — the exact
    /// query form from the ticket's report — so each label's line independently reflects
    /// rate-zero for the remainder of the range instead of just its own group stopping early.
    #[test]
    fn sum_by_rate_gets_boundary_points_per_group_across_a_wider_range() {
        let mut index = BTreeMap::new();
        declare_requests_metric(&mut index);
        ingest_log(&mut index, 0, "request path=/a");
        ingest_log(&mut index, 1, "request path=/a");
        ingest_log(&mut index, 0, "request path=/b");

        let parsed = promql::parse_query("sum(rate(requests[2s])) by (path)").unwrap();
        let mut series = evaluate_promql(&index, &parsed, None, Some(100_000)).unwrap();
        series.sort_by(|a, b| a.labels.get("path").cmp(&b.labels.get("path")));

        assert_eq!(series.len(), 2);
        assert_eq!(series[0].points, vec![(0, 0.5), (1, 1.0), (100_000, 0.0)]);
        assert_eq!(series[1].points, vec![(0, 0.5), (100_000, 0.0)]);
    }

    /// A boundary point is only appended when the real series doesn't already reach that edge —
    /// continuously-occurring metrics (the common case) must render exactly as before.
    #[test]
    fn rate_series_skips_boundary_points_already_covered_by_real_data() {
        let mut index = BTreeMap::new();
        declare_requests_metric(&mut index);
        ingest_log(&mut index, 0, "request path=/a");
        ingest_log(&mut index, 1000, "request path=/a");

        let parsed = promql::parse_query("rate(requests[2s])").unwrap();
        let series = evaluate_promql(&index, &parsed, Some(0), Some(1000)).unwrap();

        assert_eq!(series[0].points, vec![(0, 0.5), (1000, 1.0)]);
    }

    /// Ticket 040 acceptance criterion: a dashboard `rate()` chart and an alert `Threshold`
    /// condition over the same metric/window agree on the computed rate for the same historical
    /// data, since both now go through the single shared implementation in `crate::rate` rather
    /// than each scanning/windowing independently.
    #[test]
    fn dashboard_rate_and_alert_threshold_agree_on_same_windowed_data() {
        let mut index = BTreeMap::new();
        declare_requests_metric(&mut index);

        // Five matching lines 500ms apart; `ingest_log`'s `sequence` doubles as the millisecond
        // timestamp (see its definition above), so these are also the log timestamps.
        let lines: Vec<(u64, &str)> = vec![
            (0, "request path=/a"),
            (500, "request path=/a"),
            (1000, "request path=/a"),
            (1500, "request path=/a"),
            (2000, "request path=/a"),
        ];
        for (ms, text) in &lines {
            ingest_log(&mut index, *ms, text);
        }

        // Dashboard side: rate() over a 2s window, read at the series' own last timestamp.
        let parsed = promql::parse_query("rate(requests[2s])").unwrap();
        let series = evaluate_promql(&index, &parsed, None, None).unwrap();
        assert_eq!(series.len(), 1);
        let (_, dashboard_rate) = *series[0].points.last().unwrap();

        // Alert side: a Threshold condition over the same query/window, replayed over the same
        // lines in the same order. threshold=0/op=Ge always fires, so every fired message's
        // reported match count is available to compare against the dashboard's rate.
        let rule = alerts::AlertRule {
            id: "r1".to_string(),
            name: "spike".to_string(),
            enabled: true,
            condition: alerts::AlertCondition::Threshold {
                query: r#"regex("request path=/a")"#.to_string(),
                op: CompareOp::Ge,
                threshold: 0.0,
                window_ms: 2_000,
            },
        };
        let mut alert_states = HashMap::new();
        let mut last_message = String::new();
        for (ms, text) in &lines {
            let timestamp = Timestamp::from_millis(*ms as i64);
            let fired = alerts::evaluate_alerts(&[rule.clone()], &mut alert_states, &index, *ms, timestamp, text);
            assert_eq!(fired.len(), 1);
            last_message = fired[0].1.clone();
        }
        let alert_count: f64 = last_message
            .split("match count ")
            .nth(1)
            .and_then(|rest| rest.split_whitespace().next())
            .and_then(|token| token.parse().ok())
            .expect("fired message should report a match count");

        let window_seconds = 2.0;
        assert_eq!(dashboard_rate, 2.0);
        assert_eq!(alert_count, 4.0);
        assert_eq!(alert_count, dashboard_rate * window_seconds);
    }

    #[test]
    fn sum_of_rate_without_by_clause_still_collapses_to_one_series() {
        let mut index = BTreeMap::new();
        declare_requests_metric(&mut index);
        ingest_log(&mut index, 0, "request path=/a");
        ingest_log(&mut index, 0, "request path=/b");

        let parsed = promql::parse_query("sum(rate(requests[2s]))").unwrap();
        let series = evaluate_promql(&index, &parsed, None, None).unwrap();

        assert_eq!(series.len(), 1);
        assert!(series[0].labels.is_empty());
        assert_eq!(series[0].points, vec![(0, 1.0)]);
    }

    #[test]
    fn promql_range_bounds_filter_points_via_start_end_ms() {
        let mut index = BTreeMap::new();
        declare_requests_metric(&mut index);
        ingest_log(&mut index, 0, "request path=/a");
        ingest_log(&mut index, 5, "request path=/a");
        ingest_log(&mut index, 10, "request path=/a");

        let parsed = promql::parse_query("requests").unwrap();
        let series = evaluate_promql(&index, &parsed, Some(1), Some(9)).unwrap();

        assert_eq!(series.len(), 1);
        assert_eq!(series[0].points, vec![(5, 1.0)]);
    }

    #[test]
    fn declares_metric_with_value_param() {
        let mut index = BTreeMap::new();
        let pattern = r"request path=(?P<path>\S+) duration=(?P<duration>\d+)";
        declare_test_param(&mut index, "path", pattern, "path", MetricPrimitiveKind::String);
        declare_test_param(&mut index, "duration", pattern, "duration", MetricPrimitiveKind::Number);
        let result = declare_metric(
            &mut index,
            metric_request_with_value("durations", pattern, vec!["path"], Some("duration")),
        );
        assert!(result.is_ok());
    }

    // A metric may declare at most one value param — enforced at the type level now
    // (`DeclareMetricRequest::value_param` is a single `Option<String>`, not a list of
    // role-tagged params), so there's no longer a runtime "too many value params" rejection to test.

    #[test]
    fn value_param_becomes_the_bucket_value_not_a_label() {
        let mut index = BTreeMap::new();
        let pattern = r"request path=(?P<path>\S+) duration=(?P<duration>\d+)";
        declare_test_param(&mut index, "path", pattern, "path", MetricPrimitiveKind::String);
        declare_test_param(&mut index, "duration", pattern, "duration", MetricPrimitiveKind::Number);
        declare_metric(
            &mut index,
            metric_request_with_value("durations", pattern, vec!["path"], Some("duration")),
        )
        .unwrap();

        ingest_log(&mut index, 0, "request path=/a duration=5");

        let values: Vec<_> = bucket_values(&index, "durations", None).collect();
        assert_eq!(values.len(), 1);
        let (labels, (_, _, value)) = &values[0];
        assert!(!labels.contains_key("duration"));
        assert_eq!(labels.get("path").map(|v| v.to_string()), Some("/a".to_string()));
        assert_eq!(*value, MetricPrimitive::Number(Number::Int(5)));
    }

    fn declare_durations_metric(index: &mut BTreeMap<IndexKey, DataValue>) {
        let pattern = r"request path=(?P<path>\S+) duration=(?P<duration>\d+)";
        declare_test_param(index, "path", pattern, "path", MetricPrimitiveKind::String);
        declare_test_param(index, "duration", pattern, "duration", MetricPrimitiveKind::Number);
        declare_metric(
            index,
            metric_request_with_value("durations", pattern, vec!["path"], Some("duration")),
        )
        .unwrap();
    }

    #[test]
    fn gauge_returns_most_recent_value_per_label_combination() {
        let mut index = BTreeMap::new();
        declare_durations_metric(&mut index);
        ingest_log(&mut index, 0, "request path=/a duration=5");
        ingest_log(&mut index, 1, "request path=/a duration=9");
        ingest_log(&mut index, 2, "request path=/b duration=42");

        let parsed = promql::parse_query("gauge(durations)").unwrap();
        let mut series = evaluate_promql(&index, &parsed, None, None).unwrap();
        series.sort_by(|a, b| a.labels.get("path").cmp(&b.labels.get("path")));

        assert_eq!(series.len(), 2);
        assert_eq!(series[0].labels.get("path").map(String::as_str), Some("/a"));
        assert_eq!(series[0].points, vec![(1, 9.0)]);
        assert_eq!(series[1].labels.get("path").map(String::as_str), Some("/b"));
        assert_eq!(series[1].points, vec![(2, 42.0)]);
    }

    #[test]
    fn gauge_respects_time_range_bounds() {
        let mut index = BTreeMap::new();
        declare_durations_metric(&mut index);
        ingest_log(&mut index, 0, "request path=/a duration=5");
        ingest_log(&mut index, 5, "request path=/a duration=9");
        ingest_log(&mut index, 10, "request path=/a duration=42");

        let parsed = promql::parse_query("gauge(durations)").unwrap();
        let series = evaluate_promql(&index, &parsed, None, Some(9)).unwrap();

        assert_eq!(series.len(), 1);
        assert_eq!(series[0].points, vec![(5, 9.0)]);
    }

    #[test]
    fn histogram_buckets_captured_values_by_fixed_width() {
        let mut index = BTreeMap::new();
        declare_durations_metric(&mut index);
        for (seq, duration) in [(0, 2), (1, 4), (2, 12), (3, 15), (4, 21)] {
            ingest_log(&mut index, seq, &format!("request path=/a duration={duration}"));
        }

        let parsed = promql::parse_query("histogram(durations[10])").unwrap();
        let mut series = evaluate_promql(&index, &parsed, None, None).unwrap();
        series.sort_by(|a, b| a.labels.get("le").cmp(&b.labels.get("le")));

        // durations 2,4 -> le=10 (count 2); 12,15 -> le=20 (count 2); 21 -> le=30 (count 1).
        assert_eq!(series.len(), 3);
        assert_eq!(series[0].labels.get("le").map(String::as_str), Some("10"));
        assert_eq!(series[0].points[0].1, 2.0);
        assert_eq!(series[1].labels.get("le").map(String::as_str), Some("20"));
        assert_eq!(series[1].points[0].1, 2.0);
        assert_eq!(series[2].labels.get("le").map(String::as_str), Some("30"));
        assert_eq!(series[2].points[0].1, 1.0);
    }

    #[test]
    fn histogram_with_by_clause_buckets_each_label_group_separately() {
        let mut index = BTreeMap::new();
        declare_durations_metric(&mut index);
        // path=/a: 2,4 -> le=10 (count 2); 12 -> le=20 (count 1).
        // path=/b: 21 -> le=30 (count 1).
        for (seq, path, duration) in [
            (0, "/a", 2),
            (1, "/a", 4),
            (2, "/a", 12),
            (3, "/b", 21),
        ] {
            ingest_log(&mut index, seq, &format!("request path={path} duration={duration}"));
        }

        let parsed = promql::parse_query("histogram(durations[10]) by (path)").unwrap();
        let mut series = evaluate_promql(&index, &parsed, None, None).unwrap();
        series.sort_by(|a, b| {
            a.labels.get("path").cmp(&b.labels.get("path")).then(a.labels.get("le").cmp(&b.labels.get("le")))
        });

        assert_eq!(series.len(), 3);

        assert_eq!(series[0].labels.get("path").map(String::as_str), Some("/a"));
        assert_eq!(series[0].labels.get("le").map(String::as_str), Some("10"));
        assert_eq!(series[0].points[0].1, 2.0);

        assert_eq!(series[1].labels.get("path").map(String::as_str), Some("/a"));
        assert_eq!(series[1].labels.get("le").map(String::as_str), Some("20"));
        assert_eq!(series[1].points[0].1, 1.0);

        assert_eq!(series[2].labels.get("path").map(String::as_str), Some("/b"));
        assert_eq!(series[2].labels.get("le").map(String::as_str), Some("30"));
        assert_eq!(series[2].points[0].1, 1.0);
    }

    fn declare_login_duration_metric(index: &mut BTreeMap<IndexKey, DataValue>) {
        let pattern = r"user=(?P<user>\S+) logged in";
        declare_test_param(index, "user", pattern, "user", MetricPrimitiveKind::String);
        declare_elapsed_since_last_param(index, "gap");
        declare_metric(index, metric_request_with_value("login_gap", pattern, vec!["user"], Some("gap"))).unwrap();
    }

    #[test]
    fn duration_value_param_computes_elapsed_time_since_previous_same_labels_event() {
        let mut index = BTreeMap::new();
        declare_login_duration_metric(&mut index);

        ingest_log(&mut index, 0, "user=alice logged in");
        ingest_log(&mut index, 1, "user=bob logged in");
        ingest_log(&mut index, 2, "user=alice logged in");

        let values: Vec<_> = bucket_values(&index, "login_gap", None).collect();
        // First occurrences (alice@0, bob@1) produce no duration value — only alice's second occurrence (seq 2) has something to diff against.
        assert_eq!(values.len(), 1);
        let (labels, (sequence, timestamp, value)) = &values[0];
        assert_eq!(labels.get("user").map(|v| v.to_string()), Some("alice".to_string()));
        assert_eq!(*sequence, 2);
        // `ingest_log`'s helper timestamps by `sequence as millis`.
        assert_eq!(timestamp.as_millis(), 2);
        assert_eq!(*value, MetricPrimitive::Duration(2));
    }

    #[test]
    fn duration_diffs_against_the_immediately_preceding_event_not_the_first() {
        let mut index = BTreeMap::new();
        declare_login_duration_metric(&mut index);

        ingest_log(&mut index, 0, "user=alice logged in");
        ingest_log(&mut index, 3, "user=alice logged in");
        ingest_log(&mut index, 9, "user=alice logged in");

        let values: Vec<_> = bucket_values(&index, "login_gap", None).collect();
        assert_eq!(values.len(), 2);
        assert_eq!(values[0].1.2, MetricPrimitive::Duration(3));
        assert_eq!(values[1].1.2, MetricPrimitive::Duration(6));
    }

    #[test]
    fn duration_values_are_queryable_via_promql_like_any_other_metric() {
        let mut index = BTreeMap::new();
        declare_login_duration_metric(&mut index);
        ingest_log(&mut index, 0, "user=alice logged in");
        ingest_log(&mut index, 10, "user=alice logged in");

        let parsed = promql::parse_query("login_gap").unwrap();
        let series = evaluate_promql(&index, &parsed, None, None).unwrap();

        assert_eq!(series.len(), 1);
        assert_eq!(series[0].points, vec![(10, 10.0)]);
    }

    // Fixed labels (not a captured param) so a case-insensitive match collapses into one clean "error" series regardless of source-text case.
    fn declare_error_category_metric(index: &mut BTreeMap<IndexKey, DataValue>) {
        declare_metric(
            index,
            DeclareMetricRequest {
                name: "log_category".to_string(),
                message_match_regex: r"(?i)error".to_string(),
                label_params: vec![],
                value_param: None,
                matched_labels: Some(BTreeMap::from([("category".to_string(), "error".to_string())])),
                unmatched_labels: Some(BTreeMap::from([("category".to_string(), "other".to_string())])),
            },
        )
        .unwrap();
    }

    #[test]
    fn unmatched_labels_registers_occurrence_for_non_matching_lines() {
        let mut index = BTreeMap::new();
        declare_error_category_metric(&mut index);

        ingest_log(&mut index, 0, "everything is fine");
        ingest_log(&mut index, 1, "an ERROR occurred");
        ingest_log(&mut index, 2, "still fine");

        let error_labels = BTreeMap::from([("category".to_string(), MetricPrimitive::String("error".to_string()))]);
        let error_values: Vec<_> = bucket_values(&index, "log_category", Some(&error_labels)).collect();
        assert_eq!(error_values.len(), 1);
        assert_eq!(error_values[0].1.0, 1);

        let other_labels = BTreeMap::from([("category".to_string(), MetricPrimitive::String("other".to_string()))]);
        let other_values: Vec<_> = bucket_values(&index, "log_category", Some(&other_labels)).collect();
        assert_eq!(other_values.len(), 2);
    }

    #[test]
    fn unmatched_labels_produces_two_series_from_one_bare_promql_query() {
        let mut index = BTreeMap::new();
        declare_error_category_metric(&mut index);
        ingest_log(&mut index, 0, "an ERROR occurred");
        ingest_log(&mut index, 1, "all good");
        ingest_log(&mut index, 2, "all good again");

        let parsed = promql::parse_query("log_category").unwrap();
        let mut series = evaluate_promql(&index, &parsed, None, None).unwrap();
        series.sort_by(|a, b| a.labels.get("category").cmp(&b.labels.get("category")));

        assert_eq!(series.len(), 2);
        assert_eq!(series[0].labels.get("category").map(String::as_str), Some("error"));
        assert_eq!(series[0].points.len(), 1);
        assert_eq!(series[1].labels.get("category").map(String::as_str), Some("other"));
        assert_eq!(series[1].points.len(), 2);
    }

    #[test]
    fn no_unmatched_labels_means_non_matching_lines_are_untouched() {
        let mut index = BTreeMap::new();
        declare_requests_metric(&mut index);
        ingest_log(&mut index, 0, "not a request line at all");
        assert_eq!(bucket_values(&index, "requests", None).count(), 0);
    }

    #[test]
    fn handle_query_promql_rejects_malformed_query() {
        let index = BTreeMap::new();
        let response = handle_query(
            &index,
            EngineQuery::Promql {
                query: "not a valid promql query(".to_string(),
                start_ms: None,
                end_ms: None,
            },
            0,
            &mut HashMap::new(),
        );
        match response {
            EngineResponse::Promql(Err(message)) => assert!(!message.is_empty()),
            _ => panic!("expected EngineResponse::Promql(Err(_))"),
        }
    }

    #[test]
    fn handle_query_promql_returns_series_for_valid_query() {
        let mut index = BTreeMap::new();
        declare_requests_metric(&mut index);
        ingest_log(&mut index, 0, "request path=/a");

        let response = handle_query(
            &index,
            EngineQuery::Promql {
                query: r#"requests{path="/a"}"#.to_string(),
                start_ms: None,
                end_ms: None,
            },
            0,
            &mut HashMap::new(),
        );
        match response {
            EngineResponse::Promql(Ok(series)) => {
                assert_eq!(series.len(), 1);
                assert_eq!(series[0].points, vec![(0, 1.0)]);
            }
            _ => panic!("expected EngineResponse::Promql(Ok(_))"),
        }
    }

    #[test]
    fn promql_count_over_ottyl_seq_gives_one_point_per_ingested_log_with_no_metric_declaration() {
        let mut index = BTreeMap::new();
        ingest_log(&mut index, 0, "line one");
        ingest_log(&mut index, 1, "line two");
        ingest_log(&mut index, 2, "line three");

        let parsed = promql::parse_query("count(ottyl_seq)").unwrap();
        let series = evaluate_promql(&index, &parsed, None, None).unwrap();
        assert_eq!(series.len(), 1);
        assert_eq!(series[0].points, vec![(0, 1.0), (1, 1.0), (2, 1.0)]);
    }

    #[test]
    fn promql_bare_ottyl_seq_returns_the_actual_sequence_numbers() {
        let mut index = BTreeMap::new();
        ingest_log(&mut index, 0, "line zero");
        ingest_log(&mut index, 5, "line five");

        let parsed = promql::parse_query("ottyl_seq").unwrap();
        let mut series = evaluate_promql(&index, &parsed, None, None).unwrap();
        assert_eq!(series.len(), 1);
        series[0].points.sort_by_key(|(ts, _)| *ts);
        assert_eq!(series[0].points, vec![(0, 0.0), (5, 5.0)]);
    }

    #[test]
    fn promql_bare_ottyl_ts_returns_ingest_timestamps() {
        let mut index = BTreeMap::new();
        ingest_log(&mut index, 0, "a");
        ingest_log(&mut index, 1, "b");

        let parsed = promql::parse_query("ottyl_ts").unwrap();
        let series = evaluate_promql(&index, &parsed, None, None).unwrap();
        assert_eq!(series.len(), 1);
        assert_eq!(series[0].points, vec![(0, 0.0), (1, 1.0)]);
    }

    #[test]
    fn promql_rate_over_ottyl_ts_produces_a_windowed_series_not_an_empty_one() {
        let mut index = BTreeMap::new();
        for sequence in 0..5u64 {
            ingest_log(&mut index, sequence, "line");
        }

        let parsed = promql::parse_query("rate(ottyl_ts[5s])").unwrap();
        let series = evaluate_promql(&index, &parsed, None, None).unwrap();
        assert_eq!(series.len(), 1);
        assert!(!series[0].points.is_empty(), "rate(ottyl_ts[...]) must not silently return no data");
    }

    #[test]
    fn promql_ottyl_seq_with_label_matcher_matches_nothing_since_it_has_no_labels() {
        let mut index = BTreeMap::new();
        ingest_log(&mut index, 0, "line");

        let parsed = promql::parse_query(r#"ottyl_seq{path="/a"}"#).unwrap();
        let series = evaluate_promql(&index, &parsed, None, None).unwrap();
        assert!(series.is_empty());
    }

    #[test]
    fn promql_over_ottyl_fmt_is_explicitly_rejected_not_silently_empty() {
        let mut index = BTreeMap::new();
        ingest_log(&mut index, 0, r#"{"level":"error"}"#);

        let parsed = promql::parse_query("count(ottyl_fmt)").unwrap();
        let err = evaluate_promql(&index, &parsed, None, None).unwrap_err();
        assert!(err.contains("ottyl_fmt"), "error should name the rejected metric: {err}");
        assert!(err.contains("categorical") || err.contains("filter"), "error should explain why: {err}");
    }

    #[test]
    fn handle_query_promql_returns_series_for_ottyl_seq_end_to_end() {
        let mut index = BTreeMap::new();
        ingest_log(&mut index, 0, "a");
        ingest_log(&mut index, 1, "b");

        let response = handle_query(
            &index,
            EngineQuery::Promql { query: "count(ottyl_seq)".to_string(), start_ms: None, end_ms: None },
            0,
            &mut HashMap::new(),
        );
        match response {
            EngineResponse::Promql(Ok(series)) => {
                assert_eq!(series[0].points, vec![(0, 1.0), (1, 1.0)]);
            }
            _ => panic!("expected EngineResponse::Promql(Ok(_))"),
        }
    }

    #[test]
    fn declare_param_rejects_reserved_ottyl_names() {
        let mut index = BTreeMap::new();
        for name in OTTYL_RESERVED_PARAM_NAMES {
            let err = declare_param(&mut index, request(name, "x", "x")).unwrap_err();
            assert!(err.contains("reserved"), "expected a 'reserved' error for '{name}', got: {err}");
        }
    }

    // -- FilterBar query integration --

    fn logs_query(filter: Option<&str>) -> EngineQuery {
        EngineQuery::Logs(LogsParams {
            since_sequence: None,
            start_ms: None,
            end_ms: None,
            limit: None,
            filter: filter.map(str::to_string),
            sort_by: None,
            sort_missing_fallback: None,
        })
    }

    /// Ticket 045: like `logs_query`, but with an alternate sort key and its missing-param handling.
    fn logs_query_sorted(sort_by: &str, sort_missing_fallback: Option<bool>) -> EngineQuery {
        EngineQuery::Logs(LogsParams {
            since_sequence: None,
            start_ms: None,
            end_ms: None,
            limit: None,
            filter: None,
            sort_by: Some(sort_by.to_string()),
            sort_missing_fallback,
        })
    }

    fn timeline_query(filter: Option<&str>, bucket_ms: i64) -> EngineQuery {
        EngineQuery::Timeline(TimelineParams {
            start_ms: None,
            end_ms: None,
            bucket_ms: Some(bucket_ms),
            filter: filter.map(str::to_string),
        })
    }

    #[test]
    fn logs_query_with_regex_filter_returns_only_matching_lines() {
        let mut index = BTreeMap::new();
        ingest_log(&mut index, 0, "everything is fine");
        ingest_log(&mut index, 1, "an error occurred");
        ingest_log(&mut index, 2, "still fine");
        ingest_log(&mut index, 3, "another error here");

        match handle_query(&index, logs_query(Some(r#"regex("error")"#)), 0, &mut HashMap::new()) {
            EngineResponse::Logs(entries) => {
                let sequences: Vec<u64> = entries.iter().map(|e| e.sequence).collect();
                assert_eq!(sequences, vec![1, 3]);
            }
            _ => panic!("expected Logs response"),
        }
    }

    #[test]
    fn logs_query_with_ottyl_seq_comparison_filters_by_sequence() {
        let mut index = BTreeMap::new();
        for i in 0..5u64 {
            ingest_log(&mut index, i, &format!("line {i}"));
        }

        match handle_query(&index, logs_query(Some("ottyl_seq<2")), 0, &mut HashMap::new()) {
            EngineResponse::Logs(entries) => {
                let sequences: Vec<u64> = entries.iter().map(|e| e.sequence).collect();
                assert_eq!(sequences, vec![0, 1]);
            }
            _ => panic!("expected Logs response"),
        }
    }

    #[test]
    fn logs_query_with_ottyl_ts_comparison_filters_by_timestamp() {
        let mut index = BTreeMap::new();
        for i in 0..5u64 {
            let timestamp = Timestamp::from_millis(i as i64 * 1000);
            index.insert(
                IndexKey::IngestedLog { sequence: i, timestamp },
                DataValue::LogLiteral { text: format!("line {i}") },
            );
        }

        match handle_query(&index, logs_query(Some("ottyl_ts>2000")), 0, &mut HashMap::new()) {
            EngineResponse::Logs(entries) => {
                let sequences: Vec<u64> = entries.iter().map(|e| e.sequence).collect();
                assert_eq!(sequences, vec![3, 4]);
            }
            _ => panic!("expected Logs response"),
        }
    }

    // -- Ticket 045: sort_by / sort_missing_fallback --

    #[test]
    fn logs_query_sort_by_orders_lines_by_declared_param_value() {
        let mut index = BTreeMap::new();
        declare_test_param(
            &mut index,
            "priority",
            r"priority=(?P<priority>\d+)",
            "priority",
            MetricPrimitiveKind::Number,
        );
        // Ingested (and therefore default-ordered) as sequence 0, 1, 2, with priority values
        // deliberately out of that order so a passing test can only mean the sort actually ran.
        ingest_log(&mut index, 0, "priority=3 boom");
        ingest_log(&mut index, 1, "priority=1 boom");
        ingest_log(&mut index, 2, "priority=2 boom");

        match handle_query(&index, logs_query_sorted("priority", None), 0, &mut HashMap::new()) {
            EngineResponse::Logs(entries) => {
                let sequences: Vec<u64> = entries.iter().map(|e| e.sequence).collect();
                assert_eq!(sequences, vec![1, 2, 0]);
            }
            _ => panic!("expected Logs response"),
        }
    }

    /// Seeded fixture mixing lines that carry the sort param with lines that don't, per the ticket
    /// 045 acceptance criteria: with the fallback toggle on, lines missing the param are ordered by
    /// `ottyl_seq` relative to each other (and appended after the sorted lines); with it off, they're
    /// excluded from the response entirely.
    #[test]
    fn logs_query_sort_by_missing_param_fallback_orders_by_sequence_and_appends() {
        let mut index = BTreeMap::new();
        declare_test_param(
            &mut index,
            "priority",
            r"priority=(?P<priority>\d+)",
            "priority",
            MetricPrimitiveKind::Number,
        );
        ingest_log(&mut index, 0, "priority=3 boom"); // has param
        ingest_log(&mut index, 1, "no param here"); // missing
        ingest_log(&mut index, 2, "priority=1 boom"); // has param
        ingest_log(&mut index, 3, "also missing"); // missing

        match handle_query(&index, logs_query_sorted("priority", Some(true)), 0, &mut HashMap::new()) {
            EngineResponse::Logs(entries) => {
                let sequences: Vec<u64> = entries.iter().map(|e| e.sequence).collect();
                // Lines with the param sorted ascending by value (2 then 0), followed by the
                // missing-param lines in ottyl_seq order (1 then 3).
                assert_eq!(sequences, vec![2, 0, 1, 3]);
            }
            _ => panic!("expected Logs response"),
        }
    }

    #[test]
    fn logs_query_sort_by_missing_param_drop_excludes_them() {
        let mut index = BTreeMap::new();
        declare_test_param(
            &mut index,
            "priority",
            r"priority=(?P<priority>\d+)",
            "priority",
            MetricPrimitiveKind::Number,
        );
        ingest_log(&mut index, 0, "priority=3 boom");
        ingest_log(&mut index, 1, "no param here");
        ingest_log(&mut index, 2, "priority=1 boom");
        ingest_log(&mut index, 3, "also missing");

        match handle_query(&index, logs_query_sorted("priority", Some(false)), 0, &mut HashMap::new()) {
            EngineResponse::Logs(entries) => {
                let sequences: Vec<u64> = entries.iter().map(|e| e.sequence).collect();
                assert_eq!(sequences, vec![2, 0]);
            }
            _ => panic!("expected Logs response"),
        }
    }

    #[test]
    fn logs_query_sort_by_unknown_param_returns_error() {
        let mut index = BTreeMap::new();
        ingest_log(&mut index, 0, "hello");

        match handle_query(&index, logs_query_sorted("does_not_exist", None), 0, &mut HashMap::new()) {
            EngineResponse::Error(err) => assert!(err.contains("does_not_exist")),
            _ => panic!("expected EngineResponse::Error"),
        }
    }

    /// Explicit regression check for the ticket 045 acceptance criterion that default ordering
    /// (no `sort_by`) is unchanged: same fixture as the fallback/drop tests above, but with
    /// `sort_by: None` the response stays in plain ingest (`ottyl_seq`) order.
    #[test]
    fn logs_query_without_sort_by_keeps_default_ottyl_ts_order() {
        let mut index = BTreeMap::new();
        declare_test_param(
            &mut index,
            "priority",
            r"priority=(?P<priority>\d+)",
            "priority",
            MetricPrimitiveKind::Number,
        );
        ingest_log(&mut index, 0, "priority=3 boom");
        ingest_log(&mut index, 1, "no param here");
        ingest_log(&mut index, 2, "priority=1 boom");
        ingest_log(&mut index, 3, "also missing");

        match handle_query(&index, logs_query(None), 0, &mut HashMap::new()) {
            EngineResponse::Logs(entries) => {
                let sequences: Vec<u64> = entries.iter().map(|e| e.sequence).collect();
                assert_eq!(sequences, vec![0, 1, 2, 3]);
            }
            _ => panic!("expected Logs response"),
        }
    }

    // -- Ticket 060: sort helper index --

    #[test]
    fn sort_helper_index_is_built_lazily_and_reused_across_queries() {
        let mut index = BTreeMap::new();
        declare_test_param(
            &mut index,
            "priority",
            r"priority=(?P<priority>\d+)",
            "priority",
            MetricPrimitiveKind::Number,
        );
        ingest_log(&mut index, 0, "priority=3 boom");
        ingest_log(&mut index, 1, "priority=1 boom");
        ingest_log(&mut index, 2, "priority=2 boom");

        let mut sort_helper_indexes = HashMap::new();
        assert!(!sort_helper_indexes.contains_key("priority"));

        // First query has no helper index yet — must lazily build one rather than erroring or
        // silently returning unsorted results.
        match handle_query(&index, logs_query_sorted("priority", None), 0, &mut sort_helper_indexes) {
            EngineResponse::Logs(entries) => {
                let sequences: Vec<u64> = entries.iter().map(|e| e.sequence).collect();
                assert_eq!(sequences, vec![1, 2, 0]);
            }
            _ => panic!("expected Logs response"),
        }
        assert!(sort_helper_indexes.contains_key("priority"));
        assert_eq!(sort_helper_indexes["priority"].index.len(), 3);

        // Ingest more lines directly into the already-built helper index (bypassing the full
        // `EngineMessage::Ingest` actor loop, same as every other test in this file exercises
        // `handle_query` directly) via the same incremental-update path `run()` calls on ingest,
        // then requery — the new lines must appear correctly ordered without the helper index
        // having been thrown away and rebuilt from scratch.
        ingest_log(&mut index, 3, "priority=0 boom");
        update_sort_helper_indexes_on_ingest(
            &mut sort_helper_indexes,
            &index,
            3,
            Timestamp::from_millis(3),
            "priority=0 boom",
        );
        ingest_log(&mut index, 4, "no priority here");
        update_sort_helper_indexes_on_ingest(
            &mut sort_helper_indexes,
            &index,
            4,
            Timestamp::from_millis(4),
            "no priority here",
        );
        assert_eq!(sort_helper_indexes["priority"].index.len(), 5);

        match handle_query(&index, logs_query_sorted("priority", None), 0, &mut sort_helper_indexes) {
            EngineResponse::Logs(entries) => {
                let sequences: Vec<u64> = entries.iter().map(|e| e.sequence).collect();
                // 3 (priority=0) sorts before the original three; 4 (missing the param) falls back
                // to ottyl_seq order, appended after every line that has the param.
                assert_eq!(sequences, vec![3, 1, 2, 0, 4]);
            }
            _ => panic!("expected Logs response"),
        }
    }

    #[test]
    fn sort_helper_index_incremental_update_is_a_noop_when_no_index_is_live() {
        // Guards `update_sort_helper_indexes_on_ingest`'s early return: ingest must not pay to
        // derive a param nobody has ever sorted by.
        let mut sort_helper_indexes: HashMap<String, SortHelperIndexEntry> = HashMap::new();
        let index = BTreeMap::new();
        update_sort_helper_indexes_on_ingest(&mut sort_helper_indexes, &index, 0, Timestamp::from_millis(0), "hello");
        assert!(sort_helper_indexes.is_empty());
    }

    // -- Ticket 061: purging stale sort helper indexes --

    #[test]
    fn purge_stale_sort_helper_indexes_drops_only_entries_past_the_purge_duration() {
        let mut sort_helper_indexes: HashMap<String, SortHelperIndexEntry> = HashMap::new();
        sort_helper_indexes.insert(
            "stale".to_string(),
            SortHelperIndexEntry {
                index: BTreeMap::new(),
                last_accessed: std::time::Instant::now() - SORT_HELPER_INDEX_PURGE_AFTER
                    - std::time::Duration::from_secs(1),
            },
        );
        sort_helper_indexes.insert(
            "fresh".to_string(),
            SortHelperIndexEntry { index: BTreeMap::new(), last_accessed: std::time::Instant::now() },
        );

        purge_stale_sort_helper_indexes(&mut sort_helper_indexes);

        assert!(!sort_helper_indexes.contains_key("stale"));
        assert!(sort_helper_indexes.contains_key("fresh"));
    }

    #[test]
    fn sorted_query_transparently_rebuilds_a_purged_helper_index() {
        let mut index = BTreeMap::new();
        declare_test_param(
            &mut index,
            "priority",
            r"priority=(?P<priority>\d+)",
            "priority",
            MetricPrimitiveKind::Number,
        );
        ingest_log(&mut index, 0, "priority=3 boom");
        ingest_log(&mut index, 1, "priority=1 boom");
        ingest_log(&mut index, 2, "priority=2 boom");

        let mut sort_helper_indexes = HashMap::new();
        let _ = handle_query(&index, logs_query_sorted("priority", None), 0, &mut sort_helper_indexes);
        assert!(sort_helper_indexes.contains_key("priority"));

        // Simulate ticket 061's sweep having purged this entry for staleness.
        sort_helper_indexes.remove("priority");

        match handle_query(&index, logs_query_sorted("priority", None), 0, &mut sort_helper_indexes) {
            EngineResponse::Logs(entries) => {
                let sequences: Vec<u64> = entries.iter().map(|e| e.sequence).collect();
                assert_eq!(sequences, vec![1, 2, 0]);
            }
            _ => panic!("expected Logs response"),
        }
        assert!(sort_helper_indexes.contains_key("priority"));
    }

    #[test]
    fn logs_query_with_ottyl_fmt_comparison_filters_by_detected_format_for_mixed_format_lines() {
        let mut index = BTreeMap::new();
        // Seeded fixture mixing all three detectable formats on purpose (ticket 044).
        ingest_log(&mut index, 0, r#"{"level":"error","code":500}"#);
        ingest_log(&mut index, 1, r#"ts=1788648105773 msg="entity_moved" x1=-4"#);
        ingest_log(&mut index, 2, "just some plain prose here");
        ingest_log(&mut index, 3, r#"{"level":"info","code":200}"#);

        match handle_query(&index, logs_query(Some("ottyl_fmt==json")), 0, &mut HashMap::new()) {
            EngineResponse::Logs(entries) => {
                let sequences: Vec<u64> = entries.iter().map(|e| e.sequence).collect();
                assert_eq!(sequences, vec![0, 3]);
            }
            _ => panic!("expected Logs response"),
        }

        match handle_query(&index, logs_query(Some("ottyl_fmt==logfmt")), 0, &mut HashMap::new()) {
            EngineResponse::Logs(entries) => {
                let sequences: Vec<u64> = entries.iter().map(|e| e.sequence).collect();
                assert_eq!(sequences, vec![1]);
            }
            _ => panic!("expected Logs response"),
        }

        match handle_query(&index, logs_query(Some("ottyl_fmt==none")), 0, &mut HashMap::new()) {
            EngineResponse::Logs(entries) => {
                let sequences: Vec<u64> = entries.iter().map(|e| e.sequence).collect();
                assert_eq!(sequences, vec![2]);
            }
            _ => panic!("expected Logs response"),
        }
    }

    #[test]
    fn logs_query_with_json_field_comparison_filters_by_json_field() {
        let mut index = BTreeMap::new();
        ingest_log(&mut index, 0, r#"{"level":"error","code":500}"#);
        ingest_log(&mut index, 1, r#"{"level":"info","code":200}"#);
        ingest_log(&mut index, 2, "not json at all");

        match handle_query(&index, logs_query(Some(r#"json[level]==error"#)), 0, &mut HashMap::new()) {
            EngineResponse::Logs(entries) => {
                let sequences: Vec<u64> = entries.iter().map(|e| e.sequence).collect();
                assert_eq!(sequences, vec![0]);
            }
            _ => panic!("expected Logs response"),
        }

        match handle_query(&index, logs_query(Some("json[code]>=300")), 0, &mut HashMap::new()) {
            EngineResponse::Logs(entries) => {
                let sequences: Vec<u64> = entries.iter().map(|e| e.sequence).collect();
                assert_eq!(sequences, vec![0]);
            }
            _ => panic!("expected Logs response"),
        }
    }

    #[test]
    fn logs_query_combines_regex_and_metric_comparison_with_and() {
        let mut index = BTreeMap::new();
        let pattern = r"level=(?P<level>\w+) duration=(?P<duration>\d+)";
        declare_test_param(&mut index, "level", pattern, "level", MetricPrimitiveKind::String);
        declare_metric(&mut index, metric_request("requests", pattern, vec!["level"])).unwrap();

        ingest_log(&mut index, 0, "level=error duration=1 boom");
        ingest_log(&mut index, 1, "level=info duration=2 boom");
        ingest_log(&mut index, 2, "level=error duration=3 all good");

        // Only sequence 0 has both; note comparison values are bare tokens (level==error), not quoted string literals like regex()'s argument.
        match handle_query(
            &index,
            logs_query(Some(r#"regex("boom") and requests[level]==error"#)),
            0,
            &mut HashMap::new(),
        ) {
            EngineResponse::Logs(entries) => {
                let sequences: Vec<u64> = entries.iter().map(|e| e.sequence).collect();
                assert_eq!(sequences, vec![0]);
            }
            _ => panic!("expected Logs response"),
        }

        // With `or`, sequences 0 (boom) and 2 (level=error) both match.
        match handle_query(
            &index,
            logs_query(Some(r#"regex("boom") or requests[level]==error"#)),
            0,
            &mut HashMap::new(),
        ) {
            EngineResponse::Logs(entries) => {
                let sequences: Vec<u64> = entries.iter().map(|e| e.sequence).collect();
                assert_eq!(sequences, vec![0, 1, 2]);
            }
            _ => panic!("expected Logs response"),
        }
    }

    #[test]
    fn logs_query_includes_declared_params_on_matching_lines_only() {
        let mut index = BTreeMap::new();
        declare_param(&mut index, request("level", r"level=(?P<level>\w+)", "level")).unwrap();
        ingest_log(&mut index, 0, "level=info hello");
        ingest_log(&mut index, 1, "no match here");

        let EngineResponse::Logs(entries) = handle_query(&index, logs_query(None), 0, &mut HashMap::new()) else {
            panic!("expected Logs response");
        };

        assert_eq!(entries.len(), 2);
        assert_eq!(
            entries[0].params.get("level"),
            Some(&MetricPrimitive::String("info".to_string()))
        );
        assert!(entries[1].params.get("level").is_none());
    }

    #[test]
    fn boolean_param_declares_without_capture_group_and_is_always_present() {
        let mut index = BTreeMap::new();
        let mut req = request("IsError", r"(?i)error", "");
        req.kind = MetricPrimitiveKind::Boolean;
        assert!(declare_param(&mut index, req).is_ok());

        ingest_log(&mut index, 0, "an ERROR occurred");
        ingest_log(&mut index, 1, "everything is fine");

        let EngineResponse::Logs(entries) = handle_query(&index, logs_query(None), 0, &mut HashMap::new()) else {
            panic!("expected Logs response");
        };

        assert_eq!(entries.len(), 2);
        assert_eq!(entries[0].params.get("IsError"), Some(&MetricPrimitive::Boolean(true)));
        assert_eq!(entries[1].params.get("IsError"), Some(&MetricPrimitive::Boolean(false)));
    }

    #[test]
    fn timeline_query_applies_filter_before_bucketing() {
        let mut index = BTreeMap::new();
        ingest_log(&mut index, 0, "everything is fine");
        ingest_log(&mut index, 1, "an error occurred");
        ingest_log(&mut index, 2, "another error here");

        match handle_query(&index, timeline_query(Some(r#"regex("error")"#), 1_000_000), 0, &mut HashMap::new()) {
            EngineResponse::Timeline(buckets) => {
                let total: u64 = buckets.iter().map(|b| b.count).sum();
                assert_eq!(total, 2);
            }
            _ => panic!("expected Timeline response"),
        }
    }

    #[test]
    fn malformed_filter_string_surfaces_as_engine_error() {
        let mut index = BTreeMap::new();
        ingest_log(&mut index, 0, "hello");

        match handle_query(&index, logs_query(Some("@@@ not a valid filter @@@")), 0, &mut HashMap::new()) {
            EngineResponse::Error(message) => assert!(!message.is_empty()),
            _ => panic!("expected an Error response for a malformed filter string"),
        }

        match handle_query(&index, timeline_query(Some("@@@ not a valid filter @@@"), 1000), 0, &mut HashMap::new()) {
            EngineResponse::Error(message) => assert!(!message.is_empty()),
            _ => panic!("expected an Error response for a malformed filter string"),
        }
    }

    #[test]
    fn metric_comparison_without_param_is_a_compile_error() {
        let mut index = BTreeMap::new();
        let pattern = r"level=(?P<level>\w+)";
        declare_test_param(&mut index, "level", pattern, "level", MetricPrimitiveKind::String);
        declare_metric(&mut index, metric_request("requests", pattern, vec!["level"])).unwrap();
        ingest_log(&mut index, 0, "level=error");

        match handle_query(&index, logs_query(Some("requests==5")), 0, &mut HashMap::new()) {
            EngineResponse::Error(message) => assert!(message.contains("param")),
            _ => panic!("expected an Error response for a metric comparison without [param]"),
        }
    }

    #[test]
    fn metric_comparison_with_undeclared_param_is_rejected_naming_real_label_params() {
        let mut index = BTreeMap::new();
        let pattern = r"level=(?P<level>\w+) duration=(?P<duration>\d+)";
        declare_test_param(&mut index, "level", pattern, "level", MetricPrimitiveKind::String);
        declare_test_param(&mut index, "duration", pattern, "duration", MetricPrimitiveKind::Number);
        declare_metric(&mut index, metric_request("requests", pattern, vec!["level", "duration"])).unwrap();
        ingest_log(&mut index, 0, "level=error duration=5");

        match handle_query(&index, logs_query(Some("requests[nonexistent_param]==\"x\"")), 0, &mut HashMap::new()) {
            EngineResponse::Error(message) => {
                assert!(
                    message.contains("nonexistent_param"),
                    "error should name the offending param, got: {message}"
                );
                assert!(
                    message.contains("requests"),
                    "error should name the metric, got: {message}"
                );
                assert!(
                    message.contains("level") && message.contains("duration"),
                    "error should list the metric's real declared label params, got: {message}"
                );
            }
            _ => panic!("expected an Error response for an undeclared param"),
        }
    }

    #[test]
    fn metric_comparison_with_valid_label_param_still_matches() {
        let mut index = BTreeMap::new();
        let pattern = r"level=(?P<level>\w+) duration=(?P<duration>\d+)";
        declare_test_param(&mut index, "level", pattern, "level", MetricPrimitiveKind::String);
        declare_test_param(&mut index, "duration", pattern, "duration", MetricPrimitiveKind::Number);
        declare_metric(&mut index, metric_request("requests", pattern, vec!["level", "duration"])).unwrap();

        ingest_log(&mut index, 0, "level=error duration=5");
        ingest_log(&mut index, 1, "level=info duration=2");

        match handle_query(&index, logs_query(Some("requests[level]==error")), 0, &mut HashMap::new()) {
            EngineResponse::Logs(entries) => {
                let sequences: Vec<u64> = entries.iter().map(|e| e.sequence).collect();
                assert_eq!(sequences, vec![0]);
            }
            _ => panic!("expected Logs response"),
        }
    }
}