Code Review

infer_time_field heuristics are overly broad (extract_info.rs)

let looks_like_time_field = field == "@timestamp"
    || field.contains("time")
    || field.contains("timestamp")
    || bound_is_time_like;

field.contains("time") matches response_time, uptime, idle_time, etc. A query with range: { response_time: { gte: "now-5m" } } would be incorrectly inferred as the time field even though bound_is_time_like already handles the datetime-bound case. The substring checks and bound_is_time_like are partially redundant — consider replacing the open-ended contains with more precise patterns (e.g. field.ends_with("_time") || field == "timestamp").

t_lookback ignores the query's actual time range (elastic_dsl.rs)

let t_lookback = self.t_repeat; // Default to repetition delay

For a query with "gte": "now-5m" and repetition_delay: 60, the lookback is 300s but t_lookback = 60. The SQL path derives the lookback from the query's time window; using t_repeat here will produce too-aggressive cleanup and evict data that queries still need. The actual [gte, lte] bounds from the parsed predicates should drive this value.

streaming_engine is stored but never used (elastic_dsl.rs)

#[allow(dead_code)]
streaming_engine: StreamingEngine,

In the PromQL/SQL planners, StreamingEngine affects which aggregation types are emitted. If the same distinction doesn't apply here, a comment explaining why would help. If it does apply, this is a bug.

spatial_output and rollup are always identical (elastic_dsl.rs)

let spatial = KeyByLabelNames::new(group_fields);
(spatial.clone(), spatial) // rollup == spatial_output

The comment above says "all potentially available fields become rollup when they're not in the group by" but the code sets rollup equal to spatial_output, not to the complement. Comment and implementation contradict each other — one of them needs to change.

_config parameters suggest incomplete extraction (generator.rs)

Both build_elastic_streaming_yaml and build_elastic_inference_yaml accept _config: &ElasticDSLControllerConfig but never use it. Either wire it in or drop the parameter.

Minor

• get_group_by_fields for GroupBySpec::Filters deduplicates with Vec::contains (O(n)) while collect_elastic_metadata_fields in the same file uses IndexSet — worth making consistent.
• ElasticMappingSchema::add_index consumes self (builder pattern) but ElasticIndexSchema::new doesn't — minor API surface inconsistency.

Test gaps worth filling

• Conflicting time fields across queries on the same index (error path in generator.rs)
• repetition_delay % data_ingestion_interval != 0 validation error
• infer_time_field fallback when no @timestamp-like range field is present

Most of the nontrivial logic issues (e.g. rollup and lookback window) should be addressed.