Sangeetha-Grantha

Metadata Value
Status Archived
Version 1.1.0
Last Updated 2026-09-10
Author Sangeetha Grantha Team
Document Type Archive

CSV Import Strategy Implementation - Critical Review

[!NOTE] Historical evidence: results, counts, commands, and observations below belong to the original work described here. The editorial update date is not a new test or corpus verification. For present behavior, use current feature map.

Executive Summary

This document provides a comprehensive critical review of the CSV import strategy implementation for bulk importing Krithis. The implementation spans multiple tracks (TRACK-001 through TRACK-009) and represents a significant architectural evolution from initial MVP to a production-grade system.

Overall Assessment

Strengths:

Critical Issues:

Areas for Improvement:

1. Architecture Review

1.1 Unified Dispatcher Pattern (TRACK-007)

Assessment: ✅ Excellent

The evolution from multiple polling loops to a unified dispatcher with channels is a significant architectural improvement.

Strengths:

**Code Quality:**
// BulkImportWorkerService.kt:175-234
private suspend fun runDispatcherLoop(...) {
    var currentDelay = config.pollIntervalMs
    while (scope?.isActive == true) {
        // Polls for tasks across all job types
        // Sends to appropriate channels
        // Adaptive backoff with signal interrupt
    }
}

Recommendations:

  1. Consider adding metrics for dispatcher polling frequency and channel utilization
  2. Add circuit breaker pattern if dispatcher consistently finds no tasks (indicates system issue)

1.2 Batch/Job/Task Hierarchy

Assessment: ✅ Good

The three-level hierarchy (Batch → Job → Task) provides excellent granularity for tracking and management.

Strengths:

Concerns:

Recommendations:

  1. Remove unused job types or document them as future work
  2. Consider consolidating retryable and failed into a single status with retry metadata

1.3 Service Layer Organization

Assessment: ✅ Good

Services are well-separated with clear responsibilities:

BulkImportOrchestrationService → Batch lifecycle management
BulkImportWorkerService → Background processing
EntityResolutionService → Entity matching
DeduplicationService → Duplicate detection
AutoApprovalService → Review automation
NameNormalizationService → Text normalization

Strengths:

Minor Issues:

2. Design Decisions Analysis

2.0 Clarified Requirements (2026-01) Compliance Summary

The Technical Implementation Guide includes clarified requirements from 2026-01 that supersede some original strategy document specifications:

  1. ✅ CSV Raga Column Optional: Implementation correctly treats Raga as optional; scraped raga values are authoritative
  2. ✅ URL Validation Syntax-Only: Implementation correctly performs syntax-only validation (no HEAD/GET required)
  3. ⚠️ Manifest Ingest Failure: Must mark batch as FAILED even if zero tasks created - NEEDS FIX (see Section 2.1.1)
  4. ✅ Scraping Failure Handling: CSV metadata not used in scraping stage, so requirement to discard CSV-seeded metadata is N/A

2.1 CSV Parsing Implementation

Assessment: ✅ Good (Updated per Clarified Requirements)

Implementation:

**Code Review:**
// BulkImportWorkerService.kt:739-778
private fun parseCsvManifest(path: Path): List<CsvRow> {
    // Uses CSVFormat.DEFAULT with header mapping
    // Validates required columns: "krithi", "hyperlink"
    // Basic URL validation via isValidUrl()
}

2.1.1 Manifest Ingest Failure Handling

Assessment: ⚠️ Needs Fix

Clarified Requirement (2026-01):

“Manifest ingest failures must mark the batch as FAILED, even if zero tasks were created.”

**Current Implementation:**
// BulkImportWorkerService.kt:371-382
private suspend fun failManifestTask(...) {
    dal.bulkImport.updateTaskStatus(id = task.id, status = TaskStatus.FAILED, ...)
    dal.bulkImport.updateJobStatus(id = job.id, status = TaskStatus.FAILED, ...)
    dal.bulkImport.createEvent(refType = "batch", refId = job.batchId, eventType = "MANIFEST_INGEST_FAILED", ...)
    // ❌ Missing: Batch status update to FAILED
}

Issue:

**Recommendation:**
private suspend fun failManifestTask(...) {
    // ... existing code ...
    dal.bulkImport.updateBatchStatus(id = job.batchId, status = BatchStatus.FAILED, completedAt = now)
}

Clarified Requirements (2026-01) Compliance:

Gaps vs Strategy:

Recommendations:

  1. ✅ URL accessibility validation not needed per clarified requirements
  2. ✅ Raga column optionality is correct per clarified requirements
  3. Add validation for duplicate URLs within the same CSV (currently only deduplicates by hyperlink)
  4. Fix manifest ingest failure handling to mark batch as FAILED even when zero tasks created (see Section 2.1.1)

2.2 Entity Resolution & Normalization

Assessment: ✅ Very Good

Implementation Highlights:

**Code Quality:**
// EntityResolutionService.kt:47-64
private suspend fun ensureCache() {
    // Double-check locking pattern
    // Pre-normalizes all reference entities
    // Creates O(1) lookup maps
}

Strengths:

Concerns:

  1. Cache Invalidation: No mechanism to invalidate cache when new entities are created during import
  2. Normalization Edge Cases: Some normalization rules may be too aggressive (e.g., removing all spaces from raga names)
**Example Issue:**
// NameNormalizationService.kt:50
normalized = normalized.replace(" ", "") // "Kedara Gaula" -> "kedaragaula"

This may cause false matches if canonical name is “Kedara Gaula” (with space) but imported is “Kedaragaula” (without space).

Recommendations:

  1. Add cache invalidation hook when entities are created/updated
  2. Consider preserving spaces in normalization but using fuzzy matching for comparison
  3. Add unit tests for normalization edge cases

2.3 Deduplication Service

Assessment: ⚠️ Incomplete

Current Implementation:

**Code Review:**
// DeduplicationService.kt:29-72
suspend fun findDuplicates(...) {
    // 1. Check canonical krithis
    // 2. Check staging imports
    // Missing: Intra-batch deduplication during processing
}

Gaps vs Strategy:

Recommendations:

  1. Add batch context parameter to findDuplicates for intra-batch comparison
  2. Implement incipit-based matching for stronger duplicate detection
  3. Consider pre-computing duplicate candidates during batch processing (not just at resolution stage)

2.4 Auto-Approval Service

Assessment: ⚠️ Partially Implemented

**Current Implementation:**
// AutoApprovalService.kt:16-55
suspend fun autoApproveIfHighConfidence(imported: ImportedKrithiDto) {
    // Rules:
    // - HIGH confidence composer AND raga
    // - No HIGH confidence duplicates
    // - Has minimal metadata (title + lyrics)
}

Gaps vs Strategy:

Recommendations:

  1. Implement quality scoring system as specified in strategy (Section 8.2)
  2. Make auto-approval rules configurable (via database or config file)
  3. Add audit logging for auto-approvals with reasoning

3. Code Quality Assessment

3.1 Error Handling

Assessment: ✅ Good

Strengths:

**Example:**
// BulkImportWorkerService.kt:466-494
catch (e: Exception) {
    val errorJson = buildErrorPayload(
        code = "scrape_failed",
        message = "Scrape/import failed",
        url = url,
        attempt = attempt,
        cause = e.message
    )
    // Marks as RETRYABLE or FAILED based on attempt count
}

Concerns:

Recommendations:

  1. Sanitize error messages before storing (remove stack traces in production)
  2. Implement error aggregation for batch-level reporting

3.2 Database Operations

Assessment: ✅ Good

Strengths:

Concerns:

Recommendations:

  1. Optimize deduplication query to filter by normalized title in database
  2. Consider batch inserts for task creation during manifest ingest

3.3 Concurrency & Thread Safety

Assessment: ✅ Good

Strengths:

**Code Review:**
// BulkImportWorkerService.kt:78-80
private val rateLimiterMutex = Mutex()
private var globalWindow = RateWindow()
private val perDomainWindows = mutableMapOf<String, RateWindow>()

Minor Issues:

4. Implementation Gaps

4.1 Missing Features from Strategy

Feature Strategy Reference Clarified Requirements Implementation Status Priority
Quality Scoring System Section 8.2 N/A ❌ Not Implemented HIGH
URL Accessibility Validation Section 4.1.2 Syntax-only (2026-01) ✅ Implemented Correctly MEDIUM N/A
Raga Column Required Original Strategy Optional (2026-01) ✅ Implemented Correctly MEDIUM N/A
Manifest Ingest Failure Handling N/A Must mark batch FAILED (2026-01) ⚠️ Partial HIGH
Intra-Batch Deduplication Section 4.3 N/A ⚠️ Partial MEDIUM
Configurable Auto-Approval Rules Section 4.4 N/A ⚠️ Hardcoded LOW
Quality Tier Filtering (UI) Section 4.4 N/A ❌ Not Implemented LOW
Batch Statistics Dashboard Section 4.4 N/A ⚠️ Basic Only LOW

4.2 Database Schema Gaps

Missing Columns (from Strategy Section 6.2):

Recommendations:

  1. Add quality scoring columns to imported_krithis table
  2. Calculate and store quality scores during entity resolution stage
  3. Add quality tier filtering to review UI

5. Performance & Scalability

5.1 Current Performance Characteristics

Strengths:

Bottlenecks:

  1. Entity Resolution Cache: 15-minute TTL means new entities created during import won’t be found until cache expires
  2. Deduplication Query: listImports(ImportStatus.PENDING) loads all pending imports into memory
  3. Manifest Parsing: Single-threaded CSV parsing (acceptable for current scale)

5.2 Scalability Concerns

At Scale (1,200+ entries):

Recommendations:

  1. Add database connection pooling metrics
  2. Monitor task table growth and consider archival strategy
  3. Consider horizontal scaling of workers (multiple instances)

6. Error Handling & Resilience

6.1 Retry Strategy

Assessment: ✅ Good

Implementation:

**Code Review:**
// BulkImportWorkerService.kt:417-432
if (attempt > config.maxAttempts) {
    // Marks as FAILED, increments batch failed counter
    // Triggers next stage check
}

Strengths:

6.2 Failure Isolation

Assessment: ✅ Good

Strengths:

Concerns:

Recommendations:

  1. Add circuit breaker for WebScrapingService if failure rate exceeds threshold
  2. Implement adaptive rate limiting (reduce rate on errors)

7. Testing & Quality Assurance

7.1 Test Coverage

Assessment: ⚠️ Insufficient

Current State:

Missing Tests:

  1. Unit Tests:
    • NameNormalizationService normalization rules
    • EntityResolutionService matching logic
    • DeduplicationService duplicate detection
    • CSV parsing edge cases
  2. Integration Tests:
    • End-to-end batch creation → scraping → resolution → review
    • Error recovery scenarios
    • Rate limiting behavior

Recommendations:

  1. Add unit tests for normalization service (critical for data quality)
  2. Add integration test for full import pipeline
  3. Add performance tests for large batches (100+ entries)

7.2 Manual Testing

From Tracks:

Recommendations:

  1. Document manual test results
  2. Create test data fixtures for reproducible testing
  3. Add smoke tests that run on CI/CD

8. Security Considerations

8.1 File Upload Security

Assessment: ⚠️ Needs Improvement

**Current Implementation:**
// BulkImportRoutes.kt:32-63
post {
    val multipart = call.receiveMultipart()
    // Saves file to storage/imports/
    // No file size limit
    // No file type validation beyond .csv extension
}

Security Concerns:

  1. No File Size Limit: Large CSV files could cause memory issues
  2. Path Traversal Risk: File name not sanitized before saving
  3. No Content Validation: Only checks file extension, not actual CSV content

Recommendations:

  1. Add file size limit (e.g., 10MB)
  2. Sanitize file names to prevent path traversal
  3. Validate CSV content before processing (not just extension)

8.2 URL Validation

Assessment: ✅ Correct per Clarified Requirements

Current:

Clarified Requirements (2026-01):

“URL validation during manifest ingest is syntax-only (no HEAD/GET requirement).”

Status: ✅ Implementation correctly follows clarified requirements. No changes needed.

9. Frontend Implementation Review

9.1 Bulk Import Dashboard

Assessment: ✅ Good

Strengths:

**Code Review:**
// BulkImport.tsx:89-101
useEffect(() => {
    let interval: NodeJS.Timeout;
    const isRunning = selectedBatch?.status === 'RUNNING' || selectedBatch?.status === 'PENDING';
    if (isRunning && selectedBatchId) {
        interval = setInterval(() => {
            void loadBatchDetail(selectedBatchId);
            void refreshBatches();
        }, 2000);
    }
    return () => clearInterval(interval);
}, [selectedBatch?.status, selectedBatchId]);

Concerns:

Recommendations:

  1. Add cleanup for polling on component unmount
  2. Add error handling with retry logic for failed API calls

9.2 Review Workflow Integration

Assessment: ⚠️ Partial

Current:

Gaps:

Recommendations:

  1. Add batch filter dropdown to review queue
  2. Add quality tier filtering
  3. Show batch context in review queue items

10. Recommendations Summary

10.1 Critical (Must Fix)

  1. Fix Manifest Ingest Failure Handling
    • Update failManifestTask() to mark batch as FAILED when manifest ingest fails
    • Ensure batch is marked FAILED even if zero tasks were created (per clarified requirements 2026-01)
    • This is a compliance issue with clarified requirements
  2. Implement Quality Scoring System
    • Add quality_score and quality_tier columns to imported_krithis
    • Calculate scores during entity resolution (completeness + confidence + validation)
    • Use quality tiers for auto-approval and UI filtering
  3. Fix Entity Resolution Cache Invalidation
    • Invalidate cache when new entities are created
    • Or reduce TTL to 1-2 minutes during active imports

10.2 High Priority (Should Fix)

  1. Complete Deduplication Service
    • Add intra-batch deduplication during processing
    • Implement incipit-based matching
    • Optimize database queries (filter in DB, not memory)
  2. Add Comprehensive Testing
    • Unit tests for normalization and resolution logic
    • Integration tests for full pipeline
    • Performance tests for large batches
  3. Improve Error Handling
    • Sanitize error messages (remove stack traces)
    • Add error aggregation for batch summaries
    • Implement circuit breaker for external services

10.3 Medium Priority (Nice to Have)

  1. Make Auto-Approval Configurable
    • Store rules in database or config file
    • Allow per-batch configuration
    • Add audit logging for auto-approvals
  2. Enhance Frontend
    • Add batch filter to review queue
    • Add quality tier filtering
    • Improve error handling in polling
  3. Add Observability
    • Metrics for dispatcher polling frequency
    • Channel utilization metrics
    • Batch processing time histograms
    • Error rate tracking

10.4 Low Priority (Future Work)

  1. Performance Optimizations
    • Batch inserts for task creation
    • Database query optimization for deduplication
    • Consider horizontal scaling of workers
  2. Security Hardening
    • File size limits
    • Path traversal prevention
    • Content validation for CSV files

11. Conclusion

The CSV import strategy implementation represents a solid foundation with excellent architectural decisions (unified dispatcher, entity resolution caching, comprehensive orchestration). However, several critical gaps exist between the strategy document and implementation, particularly around quality scoring, URL validation, and comprehensive deduplication.

Overall Grade: B+ (Good, with room for improvement)

Key Strengths:

Key Weaknesses:

Next Steps:

  1. URGENT: Fix manifest ingest failure handling to mark batch as FAILED (clarified requirements compliance)
  2. Prioritize implementing quality scoring system
  3. Complete deduplication service with intra-batch support
  4. Add comprehensive test suite
  5. Address security concerns in file upload

Note on Clarified Requirements (2026-01): The implementation correctly follows most clarified requirements:

Clarified Requirement Analysis:

“If scraping fails after data is presented to the user, discard CSV-seeded metadata and require a new batch.”

Current Implementation:

The implementation is production-ready for current scale but needs the above improvements before handling the full 1,200+ entry target with confidence.

12. Appendix: Code Metrics

12.1 Service Complexity

Service Lines of Code Cyclomatic Complexity (Est.) Dependencies
BulkImportWorkerService ~790 High (multiple loops, conditionals) 6 services
BulkImportOrchestrationService ~145 Low 1 DAL, 1 service
EntityResolutionService ~152 Medium 1 DAL, 1 service
DeduplicationService ~98 Low 1 DAL, 1 service
AutoApprovalService ~56 Low 2 services
NameNormalizationService ~102 Low 0

12.2 Database Schema

12.3 API Endpoints

End of Review

Section index · Documentation home · Feature status