# Code Review

> Use when validating TMDL quality, DAX BPA compliance, schema consistency, or naming conventions. Triggers: "review TMDL", "check BPA compliance", "validate model quality", "code review", "check naming conventions", "find schema issues", "validate relationships", "check summarizeBy", "review model structure", "quality gate".

- Skill: `natalinio/code-review` (Agent Skill, multi-file: 2 files)
- Install (CLI): `npx skillmds@latest add natalinio/code-review`
- Raw SKILL.md: https://api.skillmd.com/api/skills/natalinio/code-review/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: AI & ML
- Author: natalinio (https://skillmd.com/u/natalinio)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/natalinio/code-review

---


# Skill: Quality Review & TMDL Validation

## Purpose
Perform a comprehensive cross-check of ALL generated artifacts before the user opens the PBIP project in Power BI Desktop. Catching errors here prevents time-consuming troubleshooting outside VS Code.

## Prerequisites — MANDATORY
Before starting the review:
1. **Read** `.github/skills/physical-model-tmdl/references/tmdl-syntax-reference.md` for syntax validation rules.
2. **Read** `.github/references/naming-conventions.md` for naming compliance.
3. If (and ONLY if) RLS is implemented, **read** `.github/references/security-rls-best-practices.md` and validate the model follows least-privilege, explicit allow/deny behavior, and auditable security logic.
4. **Load** all generated TMDL files from `<ProjectName>/PBIP/<ProjectName>.SemanticModel/definition/`.
5. **Load** the CSV mock data files from `<ProjectName>/data/`.
6. **Verify** any uncertain TMDL syntax using `microsoft_docs_search` or `microsoft_docs_fetch` MCP tool.

## Input / Output

| | |
|---|---|
| **Input** | All TMDL files in `<ProjectName>/PBIP/<ProjectName>.SemanticModel/definition/`, all CSV files in `<ProjectName>/data/` |
| **Output** | `<ProjectName>/tests/quality_review.md` |

## Review Checklist

### 1. TMDL Syntax Validation
- [ ] **Indentation**: ALL TMDL files use TAB characters (not spaces). Verify by checking for `\t`.
- [ ] **Root-level objects**: `table`, `relationship`, `expression`, `model`, `database` have NO indentation.
- [ ] **Properties**: Indented ONE tab from their parent object.
- [ ] **Multi-line expressions**: Indented TWO tabs from the parent object declaration.
- [ ] **Blank lines**: Used to separate sibling objects (measures, columns) but NOT between a property and its parent.
- [ ] **Colons** (`:`) used for non-expression properties only.
- [ ] **Equals** (`=`) used for expressions/default properties only.
- [ ] **Object names**: Enclosed in single quotes if containing spaces, dots, equals, colons, or single quotes.
- [ ] **No trailing whitespace** on any line.
- [ ] **No mixed indentation** (tabs and spaces mixed on same line).

### 2. Structural Integrity
- [ ] `model.tmdl` contains `ref table` entries for ALL tables defined in `tables/` folder.
- [ ] **CRITICAL**: `model.tmdl` contains `defaultPowerBIDataSourceVersion: powerBI_V3` property. Without it, Power BI Desktop throws *"A data model with version 3 of metadata is required"* and all refresh operations fail.
- [ ] `database.tmdl` exists with correct `compatibilityLevel` matching Power BI Desktop version (1600 for December 2025, 1567 for September 2024).
- [ ] `relationships.tmdl` contains entries for ALL FK-PK relationships.
- [ ] One `.tmdl` file per table exists in `tables/` folder.
- [ ] `_Measures.tmdl` table file exists with at least one measure.
- [ ] `expressions.tmdl` exists if shared M parameters are used.

### 3. Relationship Validation
For EACH relationship in `relationships.tmdl`:
- [ ] `fromColumn` references an existing column in an existing Fact table.
- [ ] `toColumn` references an existing column in an existing Dimension table (must have `isKey`).
- [ ] Relationship is **single-directional** (unless explicitly bi-directional for RLS).
- [ ] **CRITICAL**: `securityFilteringBehavior: oneDirection` for ALL relationships (unless specific RLS requirement). Max 1 `bothDirections` per table.
- [ ] `crossFilteringBehavior: oneDirection` for standard Star Schema (Dim → Fact).
- [ ] No **duplicate relationships** between the same column pairs.
- [ ] No **circular dependency** chains (A→B→C→A).
- [ ] FK columns in Fact tables are marked `isHidden`.
- [ ] **CRITICAL: No ambiguous paths** — For EACH pair of tables, verify there is ONLY ONE active relationship path. Common ambiguities:
  - [ ] Fact_Sales → Dim_Country (direct) AND Fact_Sales → Dim_Customer → Dim_Country (indirect)
  - [ ] Fact_Sales → Dim_Area (direct) AND Fact_Sales → Dim_Customer → Dim_Country → Dim_Area (indirect)
  - [ ] Fact_Sales → Dim_Industry (direct) AND Fact_Sales → Dim_Customer → Dim_Industry (indirect)
  - **Fix**: Remove redundant direct relationships from fact tables. Keep only the path through the most granular dimension.

### 3.1 Field Parameter Semantic Review
- [ ] Dimension parameter tables are disconnected and contain only user-facing fields that are safe to swap within the same visual role.
- [ ] Measure parameter tables are disconnected and contain only measures that are semantically comparable within the same visual.
- [ ] For each measure parameter, the dimensions used in the consuming visual are reachable through active relationships for **all** selectable measures.
- [ ] If a measure parameter mixes measures from different fact tables, the visual context is intentionally restricted to shared conformed dimensions or explicitly documented reconciliation logic.
- [ ] Reject any measure-switch design where a selected dimension produces ambiguous, partial, or misleading filtering for one or more selectable measures.

### 4. Data Type Consistency
For EACH column across all tables:
- [ ] `dataType` matches expected type (int64 for keys, decimal for amounts, string for text, dateTime for dates).
- [ ] `sourceColumn` matches the actual column name in the CSV file header.
- [ ] `summarizeBy` is set to `none` for keys and non-additive columns.
- [ ] `formatString` is appropriate (currency formats for amounts, percentage for ratios, date format for dates).

### 5. DAX Measures Validation
For EACH measure in `_Measures.tmdl`:
- [ ] Uses **VAR / RETURN** pattern for non-trivial measures.
- [ ] **VAR names do NOT use DAX reserved keywords** (`Variance`, `Status`, `Value`, `Date`, `Time`, `Year`, `Month`, `Day`, `Table`, `Column`, `Measure`, `Name`, `Count`, `Sum`, `Average`, `Min`, `Max`, `Result`, `Error`, `Number`, `Text`, `True`, `False`, `Blank`, `If`, `And`, `Or`, `Not`, `In`, `Order`, `Filter`). Use descriptive prefixes instead (e.g., `SalesBudgetVariance`, `BudgetStatusValue`).
- [ ] Uses **DIVIDE()** for ALL division operations (no `/` operator).
- [ ] ALL referenced columns exist in the model with correct `Table[Column]` syntax.
- [ ] ALL referenced measures exist and are not self-referential or circular.
- [ ] Time intelligence functions reference `Dim_Date[Date]` (the Date Table column).
- [ ] `displayFolder` is assigned to each measure.
- [ ] `lineageTag` is present and is a valid GUID.
- [ ] `formatString` is appropriate for the measure type.

### 6. Dim_Date Validation
- [ ] `Dim_Date` table contains a hidden integer surrogate key column for joins.
- [ ] A `Date` column with `dataType: dateTime` exists and is the key column after Date Table marking is persisted in Desktop.
- [ ] Fiscal year/month/quarter columns are present if fiscal periods are referenced in specs.
- [ ] The generated date range covers the full reporting period.

### 7. CSV Mock Data Validation
- [ ] ALL CSV files exist in `<ProjectName>/data/` folder.
- [ ] Files are comma-delimited and UTF-8 encoded.
- [ ] Column names in CSV headers EXACTLY match `sourceColumn` values in TMDL.
- [ ] ALL FK values in Fact table CSVs exist as PK values in corresponding Dimension CSVs (referential integrity).
- [ ] No NULL values in PK or FK columns.
- [ ] Numeric fields contain valid decimal numbers (no text in numeric columns).
- [ ] Date fields use a consistent, parseable format.

### 8. Naming Convention Compliance
- [ ] Fact table names: `Fact_<BusinessProcess>` PascalCase.
- [ ] Dimension table names: `Dim_<Entity>` PascalCase.
- [ ] Measures table: `_Measures`.
- [ ] Key columns: `<Entity>Key` PascalCase.
- [ ] Attribute columns: PascalCase with descriptive names.
- [ ] Measure names: Descriptive, following patterns in naming-conventions.md.
- [ ] CSV file names: lowercase with underscores (e.g., `fact_sales.csv`, `dim_date.csv`).
- [ ] TMDL file names: PascalCase matching table name (e.g., `Fact_Sales.tmdl`, `Dim_Date.tmdl`).

### 9. Partition Expression Validation
For EACH table's partition:
- [ ] M expression uses valid Power Query syntax.
- [ ] `File.Contents()` path points to correct CSV file location.
- [ ] `Columns` count in `Csv.Document` matches actual CSV column count.
- [ ] `Encoding` is `65001` (UTF-8).
- [ ] `Table.PromoteHeaders` is used to read CSV headers.
- [ ] `Table.TransformColumnTypes` maps types correctly (Int64.Type for integers, type number for decimals, type text for strings, type datetime for dates).

### 10. RLS Validation (if applicable)
- [ ] RLS role files exist in `roles/` subfolder.
- [ ] `tablePermission` filter expressions reference ONLY Dimension tables (never Fact tables directly).
- [ ] Filter expressions use valid DAX syntax.
- [ ] RLS logic does NOT have overly permissive fallbacks (e.g., `TRUE()` for unknown users). Prefer explicit deny-by-default behavior.
- [ ] Bi-directional cross-filtering is enabled on relationships filtered by RLS roles.

### 11. BPA Rules Validation (Detective Quality Assurance)

**Purpose**: Validate ALL generated TMDL and DAX code against Tabular Editor Best Practice Analyzer rules. This is the final quality gate before user opens PBIP in Power BI Desktop.

**Reference**: `.github/skills/code-review/references/bpa-rules-reference.md` contains all 27+ BPA rules with examples.

#### 11.1 DAX Expression Rules (Error Severity)

**Critical Rules - ZERO TOLERANCE**:

- [ ] **DAX_FULLY_QUALIFIED_COLUMNS**: ALL column references in DAX use `Table[Column]` syntax (no unqualified `[Column]`)
  - **Validation**: Grep search all measures for `\[[A-Za-z]+\]` pattern without table prefix
  - **Fix**: Replace `[Column]` with `Table[Column]`

- [ ] **DAX_DIVISION_COLUMNS**: ALL division operations use `DIVIDE()` function, NO `/` operator
  - **Validation**: Grep search all measures for `/` operator in expressions
  - **Fix**: Replace `[Numerator] / [Denominator]` with `DIVIDE([Numerator], [Denominator], 0)`

- [ ] **AVOID_RESERVED_KEYWORDS**: Object names do NOT use DAX/SQL reserved keywords (Date, Table, Value, Column, Year, etc.)
  - **Validation**: Check table/column names against reserved keyword list
  - **Fix**: Add prefix/suffix (e.g., `Date` → `Dim_Date`, `Value` → `DateValue`)

- [ ] **DAX_VAR_RESERVED_KEYWORDS**: VAR names inside DAX expressions do NOT use reserved keywords (`Variance`, `Status`, `Value`, `Date`, `Time`, `Year`, `Month`, `Day`, `Table`, `Column`, etc.)
  - **Validation**: For each `VAR <name>` in measure expressions, check `<name>` against forbidden keyword list
  - **Fix**: Use descriptive prefixed names (e.g., `VAR Variance` → `VAR SalesBudgetVariance`, `VAR Status` → `VAR BudgetStatusValue`)

#### 11.2 DAX Expression Rules (Warning Severity)

- [ ] **DAX_UNQUALIFIED_MEASURES**: Measure references are unqualified (use `[Measure]` not `Table[Measure]`)
  - **Validation**: Grep search for `\[_Measures\]\.` or `Table\.` before measure names
  - **Fix**: Remove table prefix from measure references

- [ ] **DAX_TODO_COMMENTS**: No TODO/FIXME/HACK comments in measures
  - **Validation**: Grep search for `TODO|FIXME|HACK|TEMPORARY` in measure expressions
  - **Fix**: Complete implementation or remove comment if measure is production-ready

#### 11.3 Formatting Rules (Warning Severity)

- [ ] **OBJECTS_WITH_NO_FORMAT_STRING_COLUMNS**: Numeric columns have `formatString` property
  - **Validation**: Check all columns with `dataType: decimal|double|int64` (except hidden FKs)
  - **Fix**: Add `formatString: "$#,##0.00"` for currency, `"0.00%"` for percentage, `"#,##0"` for integers

- [ ] **OBJECTS_WITH_NO_FORMAT_STRING_MEASURES**: ALL measures have `formatString` property
  - **Validation**: Check all measures in `_Measures.tmdl`
  - **Fix**: Add appropriate `formatString` based on measure type

#### 11.4 Metadata Rules (Error/Warning Severity)

- [ ] **AVOID_FLOAT_DATATYPE** (Error): Numeric columns use `dataType: decimal` NOT `double`
  - **Validation**: Grep search for `dataType: double` in all table TMDL files
  - **Fix**: Replace `double` with `decimal`

- [ ] **SUMMARIZEBY_SHOULD_BE_NONE** (Warning): ALL columns have `summarizeBy: none`
  - **Validation**: Check all columns (dimension attributes and fact measures)
  - **Fix**: Add or change to `summarizeBy: none`

- [ ] **DISABLE_ATTRIBUTE_HIERARCHIES** (Info): Foreign key columns have `isAvailableInMDX: false`
  - **Validation**: Check all FK columns in fact tables
  - **Fix**: Add `isAvailableInMDX: false`

#### 11.5 Model Layout Rules (Warning Severity)

- [ ] **HIDE_FOREIGN_KEY_COLUMNS** (Warning): ALL FK columns in fact tables have `isHidden: true`
  - **Validation**: Check all columns with `Key` suffix in fact tables
  - **Fix**: Add `isHidden: true`

- [ ] **ORGANIZE_MEASURES_IN_DISPLAY_FOLDERS** (Info): Measures have `displayFolder` property
  - **Validation**: Check all measures in `_Measures.tmdl`
  - **Fix**: Add `displayFolder: "Category\\Subcategory"` for logical grouping

- [ ] **PROVIDE_DESCRIPTIONS_FOR_MEASURES** (Info): Complex measures (time intelligence, statistical) have `description` property
  - **Validation**: Check measures with CALCULATE, DATESYTD, SAMEPERIODLASTYEAR, etc.
  - **Fix**: Add `description: "Business logic explanation"`

- [ ] **ORGANIZE_COLUMNS_IN_DISPLAY_FOLDERS** (Info): Dimension tables with 15+ columns use `displayFolder` for attribute grouping
  - **Validation**: Count columns per dimension table
  - **Fix**: Add `displayFolder: "Geography"` for location attributes, `"Demographics"` for personal attributes

#### 11.6 Naming Convention Rules

- [ ] **TABLE_NAME_MUST_START_WITH_PREFIX** (Warning): Tables use `Fact_`, `Dim_`, `Bridge_` prefixes
  - **Validation**: Check all table names in `model.tmdl` ref entries
  - **Fix**: Rename tables with correct prefix

- [ ] **USE_PASCALCASE_FOR_OBJECTS** (Info): Tables, columns, measures use PascalCase (no spaces, no snake_case)
  - **Validation**: Check for `column_name` or `columnname` patterns
  - **Fix**: Convert to `ColumnName` PascalCase

- [ ] **DATE_COLUMN_NAMED_DATE** (Warning): Date dimension has column named `Date` (dataType: dateTime)
  - **Validation**: Check `Dim_Date.tmdl` for column `Date`
  - **Fix**: Rename date column to `Date` or add new column

- [ ] **AVOID_PLURAL_TABLE_NAMES** (Info): Tables use singular names (`Dim_Product` not `Dim_Products`)
  - **Validation**: Check for table names ending in `s` (except `Sales`, `Transactions`)
  - **Fix**: Rename to singular form

- [ ] **MEASURE_NAMING_DESCRIPTIVE** (Info): Measure names are descriptive business terms, not abbreviations
  - **Validation**: Check for measures like `SA`, `M1`, `Calc_Rev`
  - **Fix**: Rename to `Total Sales`, `Revenue`, etc.

#### 11.7 Performance Rules (Warning Severity)

- [ ] **AVOID_CALCULATED_COLUMNS_IN_LARGE_FACTS** (Warning): Large fact tables (> 1M rows) have NO calculated columns
  - **Validation**: Check fact table TMDL files for column `expression:` property
  - **Fix**: Convert calculated column to measure

- [ ] **MINIMIZE_BIDIRECTIONAL_RELATIONSHIPS** (Warning): Relationships are single-directional unless RLS required
  - **Validation**: Grep search `relationships.tmdl` for `crossFilteringBehavior: bothDirections`
  - **Fix**: Change to `crossFilteringBehavior: oneDirection` or document RLS justification

- [ ] **AVOID_MEASURES_REFERENCING_CALCULATED_COLUMNS** (Warning): Measures reference physical columns, not calculated columns
  - **Validation**: Cross-check measure column references against calculated columns in tables
  - **Fix**: Rewrite measure to calculate directly from physical columns

- [ ] **USE_VARIABLES_TO_AVOID_RECALCULATION** (Info): Measures use VAR to store repeated expressions
  - **Validation**: Check for repeated expressions in IF, SWITCH, arithmetic operations
  - **Fix**: Extract to VAR before RETURN

#### 11.8 BPA Validation Output Format

Present BPA validation results as a **severity-graded report**:

```
## BPA Rules Validation Report

### Severity Summary
- ❌ **Errors (3)**: 0 found — ZERO TOLERANCE, must fix before deployment
- ⚠️ **Warnings (2)**: 2 found — Should fix, impacts performance/maintainability
- ℹ️ **Info (1)**: 5 found — Recommendations, optional for MVP

### Error Findings (Critical)
| Rule ID | File | Object | Issue | Status |
|---------|------|--------|-------|--------|
| — | — | — | No errors found | ✅ PASS |

### Warning Findings (Important)
| Rule ID | File | Object | Issue | Fix |
|---------|------|--------|-------|-----|
| OBJECTS_WITH_NO_FORMAT_STRING_MEASURES | tables/_Measures.tmdl | 'Sales YTD' | Missing formatString | Add `formatString: "$#,##0.00"` |
| HIDE_FOREIGN_KEY_COLUMNS | tables/Fact_Sales.tmdl | ProductKey | Missing isHidden | Add `isHidden: true` |

### Info Findings (Recommendations)
| Rule ID | File | Object | Issue | Recommendation |
|---------|------|--------|-------|----------------|
| PROVIDE_DESCRIPTIONS_FOR_MEASURES | tables/_Measures.tmdl | 'Sales SPLY' | No description | Add tooltip for time intelligence |
| ORGANIZE_MEASURES_IN_DISPLAY_FOLDERS | tables/_Measures.tmdl | 'Total Quantity' | No displayFolder | Group in "Sales Metrics" |
| ... | ... | ... | ... | ... |

### Overall BPA Status: ✅ PASS / ⚠️ WARNINGS / ❌ FAIL

**Decision**: 
- ✅ PASS → Model is ready for use
- ⚠️ WARNINGS → Fix critical warnings, document exceptions
- ❌ FAIL → Fix all errors before proceeding
```

#### 11.9 BPA Auto-Fix Workflow

If BPA validation finds issues:

1. **Errors (Severity 3)**: MANDATORY fixes. Propose corrections and apply automatically upon approval.
2. **Warnings (Severity 2)**: Present list, ask user: "Fix all?" or "Fix selected?" or "Document exceptions?"
3. **Info (Severity 1)**: Present recommendations, user can defer to post-MVP.

**Auto-Fix Example**:
```
Found 3 issues:
1. [ERROR] DAX_DIVISION_COLUMNS in measure 'Profit Margin' — use DIVIDE()
2. [WARNING] HIDE_FOREIGN_KEY_COLUMNS in Fact_Sales.ProductKey — add isHidden
3. [INFO] ORGANIZE_MEASURES_IN_DISPLAY_FOLDERS in measure 'Total Sales' — add displayFolder

Proposed fixes:
- File: tables/_Measures.tmdl
  Line: measure 'Profit Margin'
  Change: SUM(Fact_Sales[Profit]) / SUM(Fact_Sales[Revenue])
  To: DIVIDE(SUM(Fact_Sales[Profit]), SUM(Fact_Sales[Revenue]), 0)

Apply all fixes? (yes/no/selective)
```

#### 11.10 BPA Validation Checklist

**Prerequisites**:
- [ ] Read `.github/skills/code-review/references/bpa-rules-reference.md` (all 27+ rules)
- [ ] Load all TMDL files (database, model, tables, relationships, measures)
- [ ] Load all CSV mock data files

**Validation Sequence**:
1. Run Error (3) validations → STOP if any found, fix before proceeding
2. Run Warning (2) validations → Document or fix
3. Run Info (1) validations → Present recommendations
4. Generate severity-graded report
5. Propose auto-fixes for errors and critical warnings
6. Apply fixes upon user approval

**MCP Verification** (for uncertain rules):
```
microsoft_docs_search("Power BI DIVIDE function DAX best practices")
microsoft_docs_search("TMDL formatString syntax examples")
microsoft_docs_fetch(<url-from-search-results>)
```

---

## Output Format

Present the review as a structured report:

```
## Quality Review Report

### Overall Status: ✅ PASS / ⚠️ WARNINGS / ❌ FAIL

| # | Category | Status | Details |
|---|----------|--------|---------|
| 1 | TMDL Syntax | ✅ | All files use correct TAB indentation |
| 2 | Structural Integrity | ✅ | All required files present |
| 3 | Relationships | ✅ | 5/5 relationships validated |
| 4 | Data Types | ✅ | All types consistent |
| 5 | DAX Measures | ✅ | 12/12 measures validated |
| 6 | Dim_Date | ✅ | Fiscal year configured correctly |
| 7 | CSV Data | ✅ | Referential integrity verified |
| 8 | Naming Conventions | ✅ | All objects compliant |
| 9 | Partition Expressions | ✅ | All M expressions valid |
| 10 | RLS | N/A | No RLS requirements |
```

### Error Detail
For each ⚠️ or ❌ finding, provide:
1. **File**: Which TMDL file contains the issue.
2. **Location**: Object name and approximate line.
3. **Issue**: Clear description of what is wrong.
4. **Fix**: Exact corrected TMDL/DAX code snippet.

## Auto-Fix Proposal
If errors are found:
1. Present each error with the proposed correction.
2. Ask the user for permission to apply all fixes.
3. Apply fixes directly to the TMDL files upon approval.

Save the quality review report to `<ProjectName>/tests/quality_review.md`.