Plan Review: Reporting Hierarchy MVP Implementation
Date: 2025-01-XXStatus: 🔴 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_typein:- 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"
- Bootstrap CSV:
- 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_typefield
- Fields:
- Bootstrap endpoint will fail INSERT operations (column not found)
- Changeset JSON parsing will include invalid field
- Data loss:
relationship_typevalues will be silently ignored or cause errors
- Option A (Recommended): Remove
relationship_typefrom plan entirely (MVP doesn’t need it) - Option B: Add
relationship_typefield toconfig_agent_hierarchyschema (requires migration)
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
notesin:- Bootstrap CSV:
...,notes - Changeset JSON:
"notes": "..." - Request schemas:
notes: Optional[str] = None
- Bootstrap CSV:
- Actual table schema: NO
notesfield
- Bootstrap endpoint will fail INSERT operations (column not found)
- Data loss: Notes will be silently ignored
- Option A (Recommended): Remove
notesfrom plan (store in audit trail viacreated_by+created_atif needed) - Option B: Add
notesfield to schema (requires migration)
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):
- Removes: periods, commas, collapses spaces
- Missing: hyphen removal
-
Plan normalization function (line 353):
- Removes: periods, commas, hyphens, collapses spaces
- 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
⚠️ WARNINGS (Non-Blocking)
WARNING #1: Two Hierarchy Table Systems Exist
Issue: Codebase has TWO different hierarchy table systems:config_agent_hierarchy- usesagent_entity_id(UUID) - Plan targets this ✅dim_agent_hierarchy/dim_agent_hierarchy_seed/dim_agent_hierarchy_admin- usesagent_id(legacy)
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_hierarchyordim_agent_hierarchy - Commission engine uses
agent_id(numeric) notagent_entity_id(UUID)
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)
- Schema Alignment: Plan correctly targets
config_agent_hierarchywithagent_entity_id✅ - SCD2 Pattern: Plan matches existing
create_agent_hierarchy()andclose_agent_hierarchy()functions ✅ - Org Scoping: Plan uses
get_org_id_from_requestpattern correctly ✅ - RBAC: Plan uses
require_admin_or_ceocorrectly ✅ - Cycle Detection: Plan uses existing
detect_circular_hierarchy()function ✅ - Decimal Rules: Plan doesn’t introduce floats (hierarchy has no money) ✅
📋 REQUIRED FIXES BEFORE IMPLEMENTATION
Priority 1 (Blocking):
-
Remove
relationship_typefield from:- Bootstrap CSV format
- Changeset JSON schema
- Request/response schemas
- Seed generator output
-
Remove
notesfield from:- Bootstrap CSV format
- Changeset JSON schema
- Request/response schemas
- Seed generator output
-
Fix normalization SQL to include hyphen removal:
Priority 2 (Documentation):
- Document that plan uses
config_agent_hierarchy(Phase 4), notdim_agent_hierarchy(legacy) - 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