---
phase: 31-fan-trap-detection
plan: 02
type: execute
wave: 2
depends_on: [31-01]
files_modified:
  - src/expand.rs
  - test/sql/phase31_fan_trap.test
  - test/sql/TEST_LIST
autonomous: true
requirements: [FAN-02, FAN-03]

must_haves:
  truths:
    - "Query aggregating a metric across a one-to-many boundary is blocked with a descriptive error"
    - "Error message names the specific relationship, metric, and tables involved"
    - "Query NOT crossing a fan-out boundary succeeds normally"
    - "Derived metrics that transitively depend on base metrics in fan-out paths are also blocked"
    - "Single-table views (no joins) never trigger fan trap detection"
    - "ONE TO ONE relationships never trigger fan trap detection (both directions safe)"
    - "Transitive fan-out through chains of relationships is detected"
  artifacts:
    - path: "src/expand.rs"
      provides: "FanTrap variant in ExpandError and check_fan_traps function"
      contains: "FanTrap"
    - path: "src/expand.rs"
      provides: "check_fan_traps function called from expand()"
      contains: "check_fan_traps"
    - path: "test/sql/phase31_fan_trap.test"
      provides: "End-to-end sqllogictest for fan trap scenarios"
    - path: "test/sql/TEST_LIST"
      provides: "phase31_fan_trap.test registered"
      contains: "phase31_fan_trap"
  key_links:
    - from: "src/expand.rs"
      to: "src/model.rs"
      via: "Reads Join.cardinality to determine fan-out direction"
      pattern: "cardinality"
    - from: "src/expand.rs"
      to: "src/graph.rs"
      via: "Uses RelationshipGraph for path finding between metric and dimension source tables"
      pattern: "RelationshipGraph::from_definition"
    - from: "src/expand.rs"
      to: "expand() function"
      via: "check_fan_traps called after resolve_joins_pkfk, before SQL generation"
      pattern: "check_fan_traps"
---

<objective>
Implement fan trap detection that blocks queries where a metric aggregates across a one-to-many boundary, and add end-to-end sqllogictest coverage.

Purpose: FAN-02 and FAN-03 require that fan trap detection is a blocking error (not a warning -- per user decision deviating from original roadmap). This prevents silently-inflated aggregation results from reaching users.

Output: `ExpandError::FanTrap` variant, `check_fan_traps()` function, fan trap check wired into `expand()`, sqllogictest covering safe and unsafe query combinations.
</objective>

<execution_context>
@/Users/paul/.claude/get-shit-done/workflows/execute-plan.md
@/Users/paul/.claude/get-shit-done/templates/summary.md
</execution_context>

<context>
@.planning/PROJECT.md
@.planning/ROADMAP.md
@.planning/STATE.md
@.planning/phases/31-fan-trap-detection/31-CONTEXT.md
@.planning/phases/31-fan-trap-detection/31-RESEARCH.md
@.planning/phases/31-fan-trap-detection/31-01-SUMMARY.md
@src/expand.rs
@src/graph.rs
@src/model.rs
@test/sql/phase30_derived_metrics.test

<interfaces>
<!-- Key types and contracts from Plan 01 that this plan depends on. -->

From src/model.rs (added by Plan 01):
```rust
#[derive(Debug, Clone, Copy, PartialEq, Eq, Default, Serialize, Deserialize)]
pub enum Cardinality {
    #[default]
    ManyToOne,
    OneToOne,
    OneToMany,
}

// On Join struct:
#[serde(default)]
pub cardinality: Cardinality,
```

From src/expand.rs (existing ExpandError enum to extend):
```rust
pub enum ExpandError {
    EmptyRequest { view_name: String },
    UnknownDimension { view_name: String, name: String, available: Vec<String>, suggestion: Option<String> },
    UnknownMetric { view_name: String, name: String, available: Vec<String>, suggestion: Option<String> },
    DuplicateDimension { view_name: String, name: String },
    DuplicateMetric { view_name: String, name: String },
}
```

From src/expand.rs (expand function structure -- fan trap check goes after line 772, before SQL generation):
```rust
// Line 771-772: Pre-compute metric expressions
let topo_order = toposort_facts(&def.facts).unwrap_or_default();
let resolved_exprs = inline_derived_metrics(&def.metrics, &def.facts, &topo_order);

// >>> INSERT check_fan_traps() CALL HERE <<<

// Line 778: Build SELECT clause
let mut sql = String::with_capacity(256);
```

From src/expand.rs (existing resolve_joins_pkfk and collect_derived_metric_source_tables):
```rust
fn resolve_joins_pkfk(def: &SemanticViewDefinition, resolved_dims: &[&Dimension], resolved_mets: &[&Metric]) -> Vec<String>;
fn collect_derived_metric_source_tables(met: &Metric, all_metrics: &[Metric]) -> Vec<String>;
```

From src/graph.rs (RelationshipGraph for path finding):
```rust
pub struct RelationshipGraph {
    pub edges: HashMap<String, Vec<String>>,
    pub reverse: HashMap<String, Vec<String>>,
    pub all_nodes: HashSet<String>,
    pub root: String,
}
```
</interfaces>
</context>

<tasks>

<task type="auto" tdd="true">
  <name>Task 1: Implement ExpandError::FanTrap and check_fan_traps function</name>
  <files>src/expand.rs</files>
  <behavior>
    - Test: Fan trap detected when metric source is on "one" side of a one-to-many edge relative to dimension source
    - Test: No fan trap for MANY TO ONE direction (safe -- many rows go to one, no duplication)
    - Test: No fan trap for ONE TO ONE (both directions safe)
    - Test: Fan trap detected through transitive chain (e.g., c -> o -> li, metric from c, dim from li, edge o->li is ONE TO MANY)
    - Test: Single-table view (no joins) returns Ok immediately
    - Test: Derived metric fan trap (source_table=None) walks transitive dependencies to find base metric source tables
    - Test: FanTrap error Display format includes view name, metric, dimension, relationship, and tables
    - Test: No fan trap when metric and dimension are from the same table
  </behavior>
  <action>
    1. Add `FanTrap` variant to `ExpandError` enum:
       ```rust
       FanTrap {
           view_name: String,
           metric_name: String,
           metric_table: String,
           dimension_name: String,
           dimension_table: String,
           relationship_name: String,
       },
       ```

    2. Add `Display` implementation for `FanTrap` in the existing `fmt::Display` match block:
       ```rust
       Self::FanTrap {
           view_name, metric_name, metric_table,
           dimension_name, dimension_table, relationship_name,
       } => {
           write!(f,
               "semantic view '{view_name}': fan trap detected -- metric '{metric_name}' \
                (table '{metric_table}') would be duplicated when joined to dimension \
                '{dimension_name}' (table '{dimension_table}') via relationship \
                '{relationship_name}' (ONE TO MANY). This would inflate aggregation results. \
                Remove the dimension, use a metric from the same table, or adjust the \
                relationship cardinality."
           )
       }
       ```

    3. Add `use crate::model::Cardinality;` import at the top of expand.rs.

    4. Implement `check_fan_traps` function:
       ```rust
       fn check_fan_traps(
           view_name: &str,
           def: &SemanticViewDefinition,
           resolved_dims: &[&crate::model::Dimension],
           resolved_mets: &[&crate::model::Metric],
       ) -> Result<(), ExpandError>
       ```
       Algorithm:
       a. Early return `Ok(())` if `def.joins.is_empty()`.
       b. Build `RelationshipGraph` from definition. If it fails, return `Ok(())` (validated at define time).
       c. Build cardinality map: `HashMap<(String, String), (Cardinality, String)>` where key is `(from_alias_lower, to_alias_lower)` and value is `(cardinality, relationship_name)`. Only include joins with non-empty `fk_columns`.
       d. For each resolved metric:
          - Get source tables: if `source_table.is_some()`, use it. If `None` (derived metric), call `collect_derived_metric_source_tables` to get all transitive base metric source tables.
          - For each metric source table `met_table`:
            - For each resolved dimension with `source_table = Some(dim_table)`:
              - If `met_table == dim_table` (case-insensitive), skip (same table, no fan-out).
              - Find the path from `met_table` to `dim_table` through the tree.
              - For each edge on the path, check if traversal is in the fan-out direction:
                - Edge `(A, B)` with `ManyToOne`: A->B is safe, B->A is fan-out
                - Edge `(A, B)` with `OneToOne`: both directions safe
                - Edge `(A, B)` with `OneToMany`: A->B is fan-out, B->A is safe
              - If any edge fans out, return `Err(ExpandError::FanTrap { ... })`.
       e. Return `Ok(())` if no fan traps found.

    5. Path finding in the tree: Since the relationship graph is validated as a tree at define time, use parent-walking. Build a `parent_map: HashMap<String, String>` from `graph.reverse` (each node has at most one parent in a tree). Walk both `met_table` and `dim_table` up to the root to find their ancestor chains, then derive the path. The path is: walk `met_table` up until reaching a common ancestor with `dim_table`, then walk down to `dim_table`. For checking edges, determine whether each step follows a forward edge `(from, to)` or a reverse edge `(to, from)` in the `cardinality_map`, and check direction accordingly.

    6. Wire `check_fan_traps` into `expand()`: after line 772 (after `inline_derived_metrics`), before building the SELECT clause:
       ```rust
       // Phase 31: Check for fan traps before generating SQL.
       check_fan_traps(view_name, def, &resolved_dims, &resolved_mets)?;
       ```

    7. Add unit tests in the existing `mod tests` -> `mod expand_tests` section:
       - `fan_trap_one_to_many_blocked`: metric from table A, dimension from table B, A->B is ONE TO MANY -> blocked
       - `fan_trap_many_to_one_safe`: metric from A, dimension from B, A->B is MANY TO ONE -> allowed
       - `fan_trap_one_to_one_safe`: ONE TO ONE in either direction -> allowed
       - `fan_trap_same_table_safe`: metric and dimension from same table -> allowed
       - `fan_trap_no_joins_safe`: single-table view, no joins -> allowed
       - `fan_trap_transitive_chain`: three-table chain, one fan-out edge in the middle -> blocked
       - `fan_trap_derived_metric_blocked`: derived metric referencing base metric that crosses fan-out -> blocked
       - `fan_trap_error_message_format`: verify Display output contains view name, metric, dimension, relationship
  </action>
  <verify>
    <automated>cd /Users/paul/Documents/Dev/Personal/duckdb-semantic-views && cargo test -- fan_trap</automated>
  </verify>
  <done>ExpandError::FanTrap variant exists, check_fan_traps blocks fan-out queries, allows safe queries, handles derived metrics transitively, all unit tests pass</done>
</task>

<task type="auto">
  <name>Task 2: End-to-end sqllogictest and full test suite</name>
  <files>test/sql/phase31_fan_trap.test, test/sql/TEST_LIST</files>
  <action>
    1. Create `test/sql/phase31_fan_trap.test` with the following test scenarios:

    **Setup tables:**
    ```sql
    CREATE TABLE orders(id INTEGER, customer_id INTEGER, region VARCHAR);
    CREATE TABLE line_items(id INTEGER, order_id INTEGER, extended_price DOUBLE, status VARCHAR);
    CREATE TABLE customers(id INTEGER, name VARCHAR, segment VARCHAR);
    ```

    **Test 1: DDL with cardinality keywords parses correctly**
    ```sql
    -- statement ok
    CREATE SEMANTIC VIEW sales AS
      TABLES (
        o AS orders PRIMARY KEY (id),
        li AS line_items PRIMARY KEY (id),
        c AS customers PRIMARY KEY (id)
      )
      RELATIONSHIPS (
        li_to_order AS li(order_id) REFERENCES o MANY TO ONE,
        order_to_customer AS o(customer_id) REFERENCES c MANY TO ONE
      )
      DIMENSIONS (
        o.region AS o.region,
        c.segment AS c.segment,
        li.status AS li.status
      )
      METRICS (
        li.revenue AS SUM(li.extended_price),
        o.order_count AS COUNT(*)
      );
    ```

    **Test 2: Safe query -- metric from li, dimension from o (MANY TO ONE direction, safe)**
    ```sql
    -- query with dimensions=['region'] and metrics=['revenue'] should succeed
    -- li.revenue (from li) + o.region (from o): li->o is MANY TO ONE, safe direction
    ```

    **Test 3: Fan trap query -- metric from o, dimension from li (ONE TO MANY direction, blocked)**
    ```sql
    -- statement error
    -- Expected error substring: fan trap detected
    -- o.order_count (from o) + li.status (from li): o->li is reverse of li->o MANY TO ONE, fan-out
    ```

    **Test 4: DDL with explicit ONE TO MANY**
    ```sql
    -- Create a view with ONE TO MANY declared
    -- Show that forward direction is blocked
    ```

    **Test 5: DDL with ONE TO ONE (always safe)**
    ```sql
    -- Create a view with ONE TO ONE
    -- Show both directions work
    ```

    **Test 6: Default cardinality (no cardinality keyword = MANY TO ONE)**
    ```sql
    -- Create a view without cardinality keywords
    -- Verify MANY TO ONE behavior (same as explicit MANY TO ONE)
    ```

    **Test 7: Fan trap with derived metric**
    ```sql
    -- Derived metric referencing a base metric that would fan out
    -- Should still be blocked
    ```

    **Test 8: Case-insensitive cardinality keywords**
    ```sql
    -- Use lowercase `many to one` in DDL, verify it works
    ```

    2. Add `test/sql/phase31_fan_trap.test` to `test/sql/TEST_LIST`.

    3. Run `just build && just test-sql` to verify sqllogictest passes.

    4. Run `just test-all` to verify full test suite (unit + proptest + sqllogictest + ducklake) passes.
  </action>
  <verify>
    <automated>cd /Users/paul/Documents/Dev/Personal/duckdb-semantic-views && just test-all</automated>
  </verify>
  <done>sqllogictest covers all fan trap scenarios (safe queries succeed, fan-out queries blocked with error), full test suite passes including all prior tests (no regressions), `just test-all` green</done>
</task>

</tasks>

<verification>
- `cargo test` passes (all unit tests including fan_trap tests)
- `just build && just test-sql` passes (all sqllogictests including phase31_fan_trap.test)
- `just test-all` passes (complete quality gate)
- Fan trap queries produce blocking error with descriptive message
- Safe queries (MANY TO ONE direction, ONE TO ONE, same table) succeed normally
- Derived metrics are checked transitively
- Old definitions without cardinality work (ManyToOne default)
</verification>

<success_criteria>
- ExpandError::FanTrap variant blocks queries crossing one-to-many boundaries
- check_fan_traps wired into expand() after join resolution
- Error message names relationship, metric, dimension, and tables
- sqllogictest covers safe + blocked scenarios
- `just test-all` passes (project quality gate per CLAUDE.md)
</success_criteria>

<output>
After completion, create `.planning/phases/31-fan-trap-detection/31-02-SUMMARY.md`
</output>
