fix(l2): resolve outer group keys (incl. __name__) over binary-op sides (#52)

crates/e2e/tests/nested.rs

@@ -12,6 +12,7 @@ use std::time::Duration;
 
 use asap_ir::intent_algebra::{
     AggIntent, ArithOp, BinaryOpKind, CompareOp, L3Expr, L3Scalar, Predicate, QueryExpr, Source,
+    VectorMatch, VectorMatchKind,
 };
 use asap_ir::types::AccuracyTarget;
 use asap_frontend_promql::lower_promql;
@@ -202,6 +203,46 @@ fn q53_outer_group_key_absent_from_nested_aggregate() {
     );
 }
 
+// #52 — an outer group key referenced by neither binary-op side (`__name__`)
+//   still resolves. Each `or` side is bound independently against its own
+//   sub-tree, so `__name__` is seeded as an inherited column on both. Each side
+//   references only `env` (its matcher), so its schema is [ts, value, env,
+//   __name__] (referenced `env` first, inherited `__name__` appended) → the
+//   outer `by (__name__)` resolves to col 3 on both sides. The `or` carries the
+//   parser's default `ignoring([])` match modifier.
+#[test]
+fn q52_outer_name_label_over_binary_op() {
+    let side = |metric: &str, env: &str| QueryExpr::Scan {
+        source: Source::TimeSeries {
+            metric: metric.into(),
+        },
+        predicates: vec![Predicate(L3Expr::Compare {
+            left: Box::new(L3Expr::Column(2)), // env
+            op: CompareOp::Eq,
+            right: Box::new(L3Expr::Literal(L3Scalar::Utf8(env.into()))),
+        })],
+        schema: metric_schema(&["env", "__name__"]),
+    };
+    let expected = agg(
+        vec![3], // __name__
+        AggIntent::Sum { col: None },
+        QueryExpr::BinaryOp {
+            op: BinaryOpKind::Or,
+            lhs: Box::new(side("metric_a", "1")),
+            rhs: Box::new(side("metric_b", "2")),
+            vector_match: Some(VectorMatch {
+                kind: VectorMatchKind::Ignoring,
+                labels: vec![],
+                grouping: None,
+            }),
+        },
+    );
+    assert_eq!(
+        lower(r#"sum by (__name__)(metric_a{env="1"} or metric_b{env="2"})"#),
+        expected,
+    );
+}
+
 // #24 — sum by job over rate over a filtered scan
 //   same schema [ts, value, job, status]; rate is label-preserving,
 //   so outer sum by job still finds job at col 2

crates/frontend-promql/tests/promql_conformance.rs

@@ -790,6 +790,34 @@ fn outer_group_key_present_after_inner_aggregate_still_resolves() {
     assert_eq!(inner_by.len(), 2);
 }
 
+#[test]
+fn outer_group_key_over_binary_op_resolves_on_both_sides() {
+    // Issue #52: an outer aggregate's group key that appears in *neither* side of
+    // a binary op — the metric-name label `__name__`, or a plain `job` — must
+    // still resolve. Each `or` side is bound independently against its own
+    // sub-tree, so the key is seeded as an inherited column on both sides.
+    let qe = ok(r#"sum by (__name__)(metric_a{env="1"} or metric_b{env="2"})"#);
+    let QueryExpr::Aggregate { by, child, .. } = &qe else {
+        panic!("expected outer Aggregate, got {qe:?}");
+    };
+    // `__name__` resolves to a single positional id against the binary op output.
+    assert_eq!(by.len(), 1, "grouped by the one `__name__` key");
+    let QueryExpr::BinaryOp { lhs, rhs, .. } = child.as_ref() else {
+        panic!("expected a BinaryOp child, got {child:?}");
+    };
+    // Both independently-bound sides carry `__name__` at the same position, so
+    // the outer group key is consistent across the union.
+    let (ls, rs) = (lhs.output_schema().unwrap(), rhs.output_schema().unwrap());
+    assert_eq!(ls.column_id("__name__"), rs.column_id("__name__"));
+    assert_eq!(ls.column_id("__name__"), Some(by[0]));
+
+    // The general case (a plain label, not just `__name__`) also lowers.
+    assert!(matches!(
+        ok("sum by (job)(metric_a or metric_b)"),
+        QueryExpr::Aggregate { .. }
+    ));
+}
+
 #[test]
 fn aggregate_over_binary_op_nests() {
     // `sum(rate(a[5m]) + rate(b[5m]))` — an aggregate whose argument is a binary

crates/l2/src/binder.rs

@@ -73,6 +73,16 @@ impl<C: SchemaCatalog> Binder<C> {
     /// per distinct name referenced anywhere in the tree — so positional
     /// `ColumnId` resolution downstream is total.
     pub fn bind(&self, tree: &LQueryExpr) -> Schema {
+        self.bind_with_inherited(tree, &[])
+    }
+
+    /// Like [`bind`](Self::bind), but also seeds `inherited` label names that are
+    /// referenced by an **enclosing** scope rather than by `tree` itself. This is
+    /// how an independently-bound `BinaryOp` side (each side re-binds against its
+    /// own sub-tree) still sees an outer aggregate's group keys — e.g. the
+    /// `__name__` / `job` in `sum by (__name__)(a or b)`, which appear in neither
+    /// side's own matchers (issue #52).
+    pub fn bind_with_inherited(&self, tree: &LQueryExpr, inherited: &[String]) -> Schema {
         let mut columns: Vec<Column> = tree
             .source_name()
             .and_then(|name| self.catalog.columns_for(name))
@@ -85,10 +95,12 @@ impl<C: SchemaCatalog> Binder<C> {
             }
         }
 
-        // Append one column per referenced-but-unknown name (group keys etc.).
-        for name in collect_referenced_columns(tree) {
-            if !columns.iter().any(|c| c.name == name) {
-                columns.push(Column::new(name, DataType::Utf8, true));
+        // Append one column per referenced-but-unknown name (group keys etc.),
+        // plus any inherited-from-enclosing-scope names.
+        let referenced = collect_referenced_columns(tree);
+        for name in referenced.iter().chain(inherited) {
+            if !columns.iter().any(|c| c.name == *name) {
+                columns.push(Column::new(name.clone(), DataType::Utf8, true));
             }
         }
 
@@ -128,7 +140,7 @@ fn push_ref_name(c: &ColumnRef, out: &mut Vec<String>) {
     }
 }
 
-fn collect_referenced_columns(tree: &LQueryExpr) -> Vec<String> {
+pub(crate) fn collect_referenced_columns(tree: &LQueryExpr) -> Vec<String> {
     fn named(expr: &L2Expr, out: &mut Vec<String>) {
         for c in expr.columns_referenced() {
             push_ref_name(c, out);
@@ -211,6 +223,18 @@ mod tests {
         assert!(schema.column_id("host").is_some());
     }
 
+    #[test]
+    fn inherited_names_are_seeded_alongside_referenced() {
+        // A `BinaryOp` side re-binds against its own sub-tree, but must still see
+        // an enclosing aggregate's group key (`__name__` / `job`) that appears in
+        // neither side's own matchers (issue #52). `bind_with_inherited` seeds it.
+        let schema = Binder::new().bind_with_inherited(&src("m"), &["__name__".into()]);
+        assert!(schema.column_id("__name__").is_some());
+        // `bind` (no inheritance) does not conjure it.
+        let plain = Binder::new().bind(&src("m"));
+        assert!(plain.column_id("__name__").is_none());
+    }
+
     #[test]
     fn custom_catalog_supplies_base_columns() {
         struct FixedCatalog;

crates/l2/src/lower.rs

@@ -15,7 +15,7 @@ use std::time::Duration;
 use thiserror::Error;
 
 use asap_ir::intent_algebra::agg_intent::AggIntent;
-use crate::binder::Binder;
+use crate::binder::{collect_referenced_columns, Binder};
 use crate::column_resolution::{
     output_schema_for_aggregate, resolve_column_refs, resolve_expr, resolve_group_keys_promql,
     ResolveError,
@@ -63,7 +63,19 @@ pub fn convert_root(
     legacy: &LQueryExpr,
     accuracy: &AccuracyTarget,
 ) -> Result<CQueryExpr, ConvertError> {
-    let fallback = Binder::new().bind(legacy);
+    convert_root_with_inherited(legacy, accuracy, &[])
+}
+
+/// [`convert_root`] with label names inherited from an enclosing scope seeded
+/// into the leaf schema. Used when re-binding a `BinaryOp` side, so an outer
+/// aggregate's group keys (`sum by (__name__)(a or b)`) resolve even though they
+/// appear in neither side's own sub-tree (issue #52).
+fn convert_root_with_inherited(
+    legacy: &LQueryExpr,
+    accuracy: &AccuracyTarget,
+    inherited: &[String],
+) -> Result<CQueryExpr, ConvertError> {
+    let fallback = Binder::new().bind_with_inherited(legacy, inherited);
     let l3 = convert(legacy, &fallback, accuracy)?;
     // Both language front ends end here, so this is the one place to normalize
     // structural differences between equivalent queries (issue #34).
@@ -511,20 +523,48 @@ pub fn convert(
             lhs,
             rhs,
             vector_match,
-        } => CQueryExpr::BinaryOp {
-            op: op.clone(),
+        } => {
             // A binary op's two sides may scan different metrics with different
             // label sets, so each branch must resolve against its OWN bound
-            // schema. `convert_root` re-runs the Binder per sub-tree; threading
-            // the parent `schema` (derived from the left leaf only) would bind
-            // the right side's columns to the wrong positions.
-            lhs: Box::new(convert_root(lhs, acc)?),
-            rhs: Box::new(convert_root(rhs, acc)?),
-            vector_match: vector_match.clone(),
-        },
+            // schema — `convert_root` re-runs the Binder per sub-tree; threading
+            // the parent `schema` (a superset over both leaves) would bind the
+            // right side's columns to the wrong positions.
+            //
+            // But an independently-bound side still has to see label names an
+            // *enclosing* node references — e.g. an outer `sum by (__name__)` /
+            // `sum by (job)` over `(a or b)` whose key is in neither side's own
+            // matchers (issue #52). `fallback` carries every name referenced in
+            // this scope; subtract the names referenced *within* the binary op
+            // itself and what remains is exactly the ancestor-referenced set to
+            // inherit — seeding a sibling side's own labels would wrongly leak
+            // them across the two branches.
+            let own = collect_referenced_columns(legacy);
+            let inherited: Vec<String> = inherited_names(fallback)
+                .into_iter()
+                .filter(|n| !own.contains(n))
+                .collect();
+            CQueryExpr::BinaryOp {
+                op: op.clone(),
+                lhs: Box::new(convert_root_with_inherited(lhs, acc, &inherited)?),
+                rhs: Box::new(convert_root_with_inherited(rhs, acc, &inherited)?),
+                vector_match: vector_match.clone(),
+            }
+        }
     })
 }
 
+/// The label names an enclosing scope's schema carries beyond the `(ts, value)`
+/// floor — the set an independently-bound `BinaryOp` side must inherit so an
+/// outer aggregate's group keys still resolve (issue #52).
+fn inherited_names(schema: &Schema) -> Vec<String> {
+    schema
+        .columns
+        .iter()
+        .filter(|c| c.name != "ts" && c.name != "value")
+        .map(|c| c.name.clone())
+        .collect()
+}
+
 /// Build a canonical `Scan`. A schema-bearing [`SourceSpec`] (SQL table) emits
 /// a `Source::Table` carrying that resolved schema; a schema-less one (PromQL)
 /// emits a `Source::TimeSeries` carrying the Binder's usage-derived `fallback`.

docs/promql-lowering.md

@@ -108,6 +108,20 @@ Why isolate this instead of resolving inline during lowering:
   rejected here: a usage-derived schema can't enumerate "all labels *except*
   these," so the error belongs in binding, not smeared across lowering.
 
+### Binary-op sides bind independently — with inherited keys
+
+Each side of a `BinaryOp` re-runs the Binder against its **own** sub-tree
+(`convert_root` per side), because the two branches may scan different metrics
+with different label sets and each side's columns must bind to positions in its
+own leaf — not the other's. But a side must still see label names referenced by
+an **enclosing** node: `sum by (__name__)(a or b)` (or `sum by (job)(a or b)`)
+groups by a key that appears in *neither* side's own matchers, and every series
+carries `__name__` (the metric name) regardless (issue #52). So the converter
+seeds those **inherited** names — the enclosing scope's referenced columns minus
+the ones referenced inside the binary op itself, i.e. exactly the ancestor keys —
+into each side via `Binder::bind_with_inherited`. Subtracting the binary op's own
+references is what keeps one side's labels from leaking into the other.
+
 ---
 
 ## Why `unique_keys` / CSE