# #1294 Phase 2 Rollup-Reference Test Migration Design

**Issue:** `#1294`

**Goal:** Remove positive in-tree test and fixture coverage that still teaches cross-child aggregate formula authoring as a supported contract, and replace it with the supported `rollupsummary` plus formula-reference contract already present on `main`.

## Scope

Phase 2 changes test and fixture coverage only. It does not remove the legacy aggregate parser/compiler/view-builder/runtime path.

In scope:

- analytics tests that currently express cross-child aggregation by authoring formula text such as `SUM(Child.Field)` or `COUNT(Child.*)`
- parity fixtures whose user-facing contract should now be "stored rollup field read through a formula view"
- moving legacy aggregate compatibility coverage to lower-layer tests that do not teach aggregate formula authoring as a supported metadata contract
- design and plan docs that make the repo-local Phase 2 boundary explicit

Out of scope:

- removing `AggregateExpr`
- removing `parseAggregateOrFunc`
- removing aggregate marker emission or aggregate JOIN marker resolution
- changing runtime behavior for already persisted legacy aggregate formulas
- adding a repo-local persisted-data migration tool without a real public-repo target
- deciding whether `AVG` remains a NexusCRM rollup delta

## Verified Current State

These facts are verified on current `origin/main` as of 2026-05-24:

- `#1294` is still open and still defines Phase 2 as migration work between Phase 1 authoring rejection and Phase 3 runtime deletion.
- The legacy aggregate path is still live in production code:
  - `AggregateExpr` exists in `formula_dsl_ast.go`
  - `parseAggregateOrFunc` exists in `formula_dsl_parser.go`
  - aggregate marker emission exists in `formula_dsl_compiler.go`
  - aggregate marker resolution exists in `formula_dsl_viewbuilder.go`
  - runtime aggregate JOIN resolution exists in `formula_view_service.go`
- The supported formula-view contract for rollup references already exists:
  - `TestBuildViewFromDSL_RollupSummaryReferenceUsesStoredColumn` in `formula_view_service_test.go` proves active formulas read the stored parent `rollupsummary` column instead of synthesizing a formula-authored aggregate JOIN
- `#871` and `#877` are both closed, so this PR does not need to invent new rollup-reference runtime behavior
- There is no current public-repo JSON payload under `backend/**` or `shared/**` that still persists formula text authoring `SUM(...)`, `COUNT(...)`, `AVG(...)`, `MIN(...)`, or `MAX(...)`

The last fact is the stop-loss boundary for Phase 2 in this repo: there is no concrete public-repo persisted-data target that can justify landing a migration tool here today.

## Authoritative Rule

Cross-child aggregation is supported only through `rollupsummary` metadata and stored parent columns.

- Supported user contract:
  - declare `Field.Type = rollupsummary`
  - configure `Field.RollupConfig`
  - reference the stored rollup field from active formulas by field name
- Unsupported user contract:
  - author cross-child aggregate formula text such as `SUM(Child.Amount)` or `COUNT(Child.*)`
- Transitional legacy runtime contract, preserved until Phase 3:
  - already persisted aggregate formulas may still compile through the old aggregate runtime path
  - that path remains an implementation compatibility layer, not a supported authoring surface

Phase 2 therefore changes what the repo teaches as supported behavior, not what the transitional runtime can still tolerate.

## High-Confidence Design

Use a two-level migration:

### 1. Move supported semantic tests to rollupsummary reference coverage

Any test whose purpose is "cross-child aggregation works for users" must stop authoring aggregate formula text and instead use:

- a stored parent rollup field
- a formula that references that stored field
- assertions framed around the stored rollup contract

Examples:

- the soft-delete parity integration test should:
  - create stored rollup columns on the parent table
  - compute rollup values through `RollupRepository`
  - write those values into the parent row
  - assert the formula view projects those stored columns correctly
- dependency tests should assert that a formula referencing a stored rollup field creates no child-view dependency edge
- view-build tests should assert that formulas reference the stored parent rollup column rather than synthesizing aggregate SQL

### 2. Push legacy aggregate compatibility coverage down to lower layers

Any remaining coverage whose purpose is "the transitional aggregate runtime machinery still behaves consistently until Phase 3" must stop driving that machinery through formula authoring strings.

Instead:

- compiler tests should construct `AggregateExpr` directly and verify aggregate marker emission
- view-builder tests should exercise aggregate marker replacement helpers directly with prebuilt markers and `AggregateJoinSpec`
- service tests that validate aggregate relationship resolution should call `resolveAggregateJoinSpec(...)` directly with constructed `AggregateExpr` values

This keeps Phase 2 aligned with the supported contract while still preserving confidence that the legacy runtime path has not silently drifted before Phase 3 deletes it.

## File-Level Plan

Primary supported-contract migration:

- `backend/internal/features/analytics/formula_view_rollup_parity_integration_test.go`
- `backend/internal/features/analytics/formula_view_service_test.go`
- `backend/internal/features/analytics/formula_view_dependencies_test.go`

Legacy-compat coverage moved lower:

- `backend/internal/features/analytics/formula_dsl_compiler_test.go`
- `backend/internal/features/analytics/formula_dsl_viewbuilder_test.go`

Likely untouched production code:

- `backend/internal/features/analytics/*.go`

Production code should change only if a tiny test-support refactor is required. Any change that widens into parser/runtime deletion belongs to Phase 3 and must stop this PR.

## Why This Is the Highest-Confidence Scope

This shape has the lowest merge risk because it matches what the repo already proves on `main`:

- Phase 1 already blocked new authoring
- the rollupsummary reference contract already has direct coverage
- the public repo has no persisted aggregate-formula payload to migrate
- the legacy runtime still exists and can keep its narrow compatibility coverage until Phase 3

Anything broader would force this PR to invent or re-design a second surface:

- adding a repo-local migration tool would be speculative because there is no public-repo data target
- deleting runtime pieces would collapse into Phase 3
- changing rollup runtime semantics would widen into unrelated substrate work

## Tests

Required verification after implementation:

- `cd backend && go test ./internal/features/analytics -count=1`
- `git diff --check`

Helpful audit after test migration:

- search for remaining positive aggregate-authoring analytics fixtures:
  - `rg -n 'Expression: \"(SUM|COUNT|AVG|MIN|MAX)\\(|Formula:\\s+.*\"(SUM|COUNT|AVG|MIN|MAX)\\(|Parse\\(\"(SUM|COUNT|AVG|MIN|MAX)\\(' backend/internal/features/analytics/*test.go`

This audit is not expected to reach zero globally because Phase 1 rejection tests and narrowly scoped legacy compatibility tests may still mention aggregate syntax. The success condition is narrower: positive supported-contract analytics tests must stop teaching aggregate formula authoring.