Skip to main content

Plan Review: Reporting Hierarchy MVP Implementation

Date: 2025-01-XX
Status: 🔴 RED FLAGS IDENTIFIED - Must fix before implementation

🔴 CRITICAL RED FLAGS

RED FLAG #1: relationship_type Field Does Not Exist

Issue: Plan references relationship_type field in multiple places, but config_agent_hierarchy table schema does NOT have this field. Evidence:
  • Plan uses relationship_type in:
    • Bootstrap CSV: child_agent_entity_id,parent_agent_entity_id,effective_start_date,relationship_type,active,notes
    • Changeset JSON: "relationship_type": "REPORTS_TO"
    • Request schemas: relationship_type: Optional[str] = "reporting"
  • Actual table schema (integration/bigquery/schemas/config_agent_hierarchy_schema.json):
    • Fields: hierarchy_id, tenant_id, org_id, agent_entity_id, parent_agent_entity_id, effective_start_date, effective_end_date, change_status, is_active, created_at, created_by, record_hash
    • NO relationship_type field
Impact:
  • Bootstrap endpoint will fail INSERT operations (column not found)
  • Changeset JSON parsing will include invalid field
  • Data loss: relationship_type values will be silently ignored or cause errors
Fix Required:
  1. Option A (Recommended): Remove relationship_type from plan entirely (MVP doesn’t need it)
  2. Option B: Add relationship_type field to config_agent_hierarchy schema (requires migration)
Recommendation: Option A - Remove relationship_type from MVP. It’s marked as “Future use” anyway. Can add later if needed.

RED FLAG #2: notes Field Does Not Exist

Issue: Plan uses notes field in bootstrap CSV and changeset JSON, but config_agent_hierarchy table does NOT have this field. Evidence:
  • Plan uses notes in:
    • Bootstrap CSV: ...,notes
    • Changeset JSON: "notes": "..."
    • Request schemas: notes: Optional[str] = None
  • Actual table schema: NO notes field
Impact:
  • Bootstrap endpoint will fail INSERT operations (column not found)
  • Data loss: Notes will be silently ignored
Fix Required:
  1. Option A (Recommended): Remove notes from plan (store in audit trail via created_by + created_at if needed)
  2. Option B: Add notes field to schema (requires migration)
Recommendation: Option A - Remove notes from MVP. Can add later if needed for audit trail.

RED FLAG #3: Normalization SQL Mismatch

Issue: Plan’s SQL query example doesn’t match the normalization function - missing hyphen removal. Evidence:
  • Plan SQL query (line 122):
    UPPER(TRIM(REPLACE(REPLACE(REPLACE(CONCAT(c.first_name, ' ', c.last_name), '.', ''), ',', ''), '  ', ' ')))
    
    • Removes: periods, commas, collapses spaces
    • Missing: hyphen removal
  • Plan normalization function (line 353):
    full.replace('.', '').replace(',', '').replace('-', '')
    
    • Removes: periods, commas, hyphens, collapses spaces
Impact:
  • Resolver endpoint will return different normalized names than seed generator expects
  • Name matching will fail for names with hyphens (e.g., “Mary-Jane” vs “MARYJANE”)
  • Fail-closed matching will incorrectly reject valid agents
Fix Required: Update SQL query to match normalization function:
UPPER(TRIM(REPLACE(REPLACE(REPLACE(REPLACE(CONCAT(c.first_name, ' ', c.last_name), '.', ''), ',', ''), '-', ''), '  ', ' '))) AS full_name_normalized

⚠️ WARNINGS (Non-Blocking)

WARNING #1: Two Hierarchy Table Systems Exist

Issue: Codebase has TWO different hierarchy table systems:
  1. config_agent_hierarchy - uses agent_entity_id (UUID) - Plan targets this
  2. dim_agent_hierarchy / dim_agent_hierarchy_seed / dim_agent_hierarchy_admin - uses agent_id (legacy)
Status: Plan correctly targets config_agent_hierarchy. No action needed, but be aware of confusion risk. Recommendation: Document clearly that plan uses config_agent_hierarchy (Phase 4), not dim_agent_hierarchy (legacy).

WARNING #2: Commission Engine Isolation Verified ✅

Status: Verified - No hierarchy joins found in commission allocation code. Evidence:
  • Commission allocation uses PEPM rates from agent2_pepm.csv
  • No queries join to config_agent_hierarchy or dim_agent_hierarchy
  • Commission engine uses agent_id (numeric) not agent_entity_id (UUID)
Recommendation: Continue monitoring - ensure no future changes add hierarchy joins to payout paths.

WARNING #3: Existing Functions Use Different Signature

Issue: api/bigquery/agent_mapping_config_queries.py has create_agent_hierarchy() that uses agent_id (legacy), but plan uses phase4_hierarchy_queries.py which uses agent_entity_id. Status: Plan correctly uses phase4_hierarchy_queries.py. No action needed. Recommendation: Ensure implementation uses phase4_hierarchy_queries.py, not agent_mapping_config_queries.py.

✅ ALIGNMENT CHECKS (Passing)

  1. Schema Alignment: Plan correctly targets config_agent_hierarchy with agent_entity_id
  2. SCD2 Pattern: Plan matches existing create_agent_hierarchy() and close_agent_hierarchy() functions ✅
  3. Org Scoping: Plan uses get_org_id_from_request pattern correctly ✅
  4. RBAC: Plan uses require_admin_or_ceo correctly ✅
  5. Cycle Detection: Plan uses existing detect_circular_hierarchy() function ✅
  6. Decimal Rules: Plan doesn’t introduce floats (hierarchy has no money) ✅

📋 REQUIRED FIXES BEFORE IMPLEMENTATION

Priority 1 (Blocking):

  1. Remove relationship_type field from:
    • Bootstrap CSV format
    • Changeset JSON schema
    • Request/response schemas
    • Seed generator output
  2. Remove notes field from:
    • Bootstrap CSV format
    • Changeset JSON schema
    • Request/response schemas
    • Seed generator output
  3. Fix normalization SQL to include hyphen removal:
    UPPER(TRIM(REPLACE(REPLACE(REPLACE(REPLACE(CONCAT(c.first_name, ' ', c.last_name), '.', ''), ',', ''), '-', ''), '  ', ' '))) AS full_name_normalized
    

Priority 2 (Documentation):

  1. Document that plan uses config_agent_hierarchy (Phase 4), not dim_agent_hierarchy (legacy)
  2. Add note about two hierarchy systems in codebase

🎯 IMPLEMENTATION READINESS

Status: ⚠️ NOT READY - Must fix 3 critical red flags before implementation Estimated Fix Time: 1-2 hours (schema updates + plan updates) Risk Level After Fixes: 🟢 LOW - All other checks pass, no downstream breakage expected

📝 NOTES

  • Commission engine isolation verified - safe to proceed after fixes
  • Existing functions align with plan expectations
  • Org-scoping patterns match codebase conventions
  • SCD2 pattern matches existing implementation