| Metadata | Value |
|---|---|
| Status | Active |
| Version | 1.1.0 |
| Last Updated | 2026-09-10 |
| Author | Sangeetha Grantha Team |
| Document Type | Evidence record |
[!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 quality checks.
Review Date: 2026-01-27
Reviewer: Claude Code Analysis
File: modules/frontend/sangita-admin-web/src/pages/KrithiEditor.tsx
File Size: ~1,860 lines Component Type: Page-level editor component (complex form) Overall Assessment: The component is functional but has significant architectural issues that impact maintainability, testability, and scalability.
The component is a single 1,860-line file handling multiple concerns:
Impact: Difficult to test, reason about, and modify without risk of regressions.
const SectionHeader: React.FC<...> = ({ title, action }) => (...)
const InputField = ({ label, value, onChange, ... }: any) => (...)
const SelectField = ({ label, value, onChange, ... }: any) => (...)
const TextareaField = ({ ... }: any) => (...)
const CheckboxField = ({ ... }: any) => (...)
Problems:
any type defeats TypeScript’s purposeReact.memoThe component has 25+ useState calls:
const [activeTab, setActiveTab] = useState(...)
const [loading, setLoading] = useState(false)
const [saving, setSaving] = useState(false)
const [sectionsLoading, setSectionsLoading] = useState(false)
const [lyricVariantsLoading, setLyricVariantsLoading] = useState(false)
const [tagsLoading, setTagsLoading] = useState(false)
// ... 20+ more
Impact: Complex state interdependencies, difficult to track state changes, prone to stale closure bugs.
any (Critical)const InputField = ({ label, value, onChange, ... }: any) => (...) // Line 57
const SelectField = ({ label, value, onChange, ... }: any) => (...) // Line 81
const TextareaField = ({ ... }: any) => (...) // Line 97
const CheckboxField = ({ ... }: any) => (...) // Line 110
const payload: any = {}; // Line 396
const mapKrithiDtoToDetail = (dto: any): Partial<KrithiDetail> => {...} // Line 199
Impact: Eliminates compile-time type checking, increases runtime errors.
workflowState as any // Line 235
e.target.value as any // Lines 1005, 1086)
tab as any // Line 746
Impact: Bypasses TypeScript safety, potential runtime errors.
The inline components lack proper interfaces. For example:
interface InputFieldProps {
label: string;
value: string;
onChange: (value: string) => void;
placeholder?: string;
highlight?: boolean;
}
The component mixes different patterns inconsistently:
// Pattern 1: Spread operator
setKrithi({ ...krithi, title: v }) // Line 822
// Pattern 2: Functional update
setKrithi(prev => ({ ...prev, composer: obj })) // Line 326
Best practice: Always use functional updates when the new state depends on previous state.
Values computed from state are recalculated on every render:
krithi.sections?.map(...)
krithi.lyricVariants?.map(...)
Should use useMemo for expensive computations.
const [rawKrithiDto, setRawKrithiDto] = useState<any>(null); // Line 245
This stores raw API response to remap later - indicates a design issue with the data loading strategy.
Multiple interdependent effects create a waterfall of side effects:
// Effect 1: Load reference data (Lines 178-196)
useEffect(() => { loadRefs(); }, []);
// Effect 2: Load krithi (Lines 247-268)
useEffect(() => {...}, [krithiId, isNew]);
// Effect 3: Remap on reference data change (Lines 272-284)
useEffect(() => {...}, [rawKrithiDto, composers, ragas, ...]);
// Effect 4: Auto-load sections (Lines 287-316)
useEffect(() => {...}, [krithiId, isNew]);
// Effect 5: Reset sections flag (Lines 319-321)
useEffect(() => { sectionsLoadedRef.current = false; }, [krithiId]);
Impact: Hard to trace data flow, potential for race conditions, difficult to debug.
useEffect(() => {
// Uses `composers`, `ragas`, `talas`, `deities`, `temples`
// but they're not in deps (intentional but fragile)
}, [krithiId, isNew]); // Line 268
useEffect(() => {
getKrithi(krithiId).then(...) // No AbortController
}, [krithiId]);
Impact: Potential memory leaks, race conditions with rapid navigation.
The mapKrithiDtoToDetail function contains complex mapping logic that should be in a separate utility:
const mapKrithiDtoToDetail = (dto: any): Partial<KrithiDetail> => {
// 40+ lines of transformation logic
let workflowState = 'DRAFT';
if (dto.workflowState) {
workflowState = typeof dto.workflowState === 'string'
? dto.workflowState.toUpperCase().replace(/-/g, '_')
: dto.workflowState;
}
// ... extensive mapping
}
Impact: Untestable, duplicated concepts, business logic mixed with UI.
The handleSave function is ~280 lines containing:
This should be decomposed into smaller, testable functions.
onClick={async () => {
setActiveTab(tab as any);
if (tab === 'Structure' && ...) {
// 20+ lines of async data loading
}
if (tab === 'Lyrics' && ...) {
// 20+ lines of async data loading
}
}}
Impact: Tab clicks trigger side effects, making behavior unpredictable and hard to test.
Different loading patterns for different data:
{loading ? <Spinner /> : <Content />} // Main loading
{sectionsLoading && <Spinner />} // Sections
{lyricVariantsLoading ? ... : ...} // Variants (ternary)
{tagsLoading && <Spinner />} // Tags
const timer = setTimeout(async () => {
// Load sections after delay to ensure krithi state is set
}, 100); // Line 313
Impact: Race condition workaround, fragile timing dependency.
.catch(err => alert("Failed to load krithi: " + err.message)) // Line 260 - alert
.catch(err => console.error("Failed to load audit logs:", err)) // Line 266 - console
toast.error('Save failed: ' + (e.message || 'Unknown error')) // Line 670 - toast
} catch (err: any) {
console.error('Failed to load sections:', err);
// Don't show error toast on initial load - sections might not exist yet
} // Lines 307-309
Impact: User doesn’t know when operations fail.
All inline components re-render on every state change.
Handlers like handleSave, handleComposerChange, etc. are recreated every render:
const handleComposerChange = (id: string) => {...} // Line 324
Rendering 6 tabs’ content conditionally but all tab content is evaluated:
{activeTab === 'Metadata' && (...)} // ~200 lines
{activeTab === 'Structure' && (...)} // ~130 lines
{activeTab === 'Lyrics' && (...)} // ~220 lines
// etc.
role="tablist", role="tab", role="tabpanel"aria-modal="true", aria-labelledbyaria-describedby for error stateskrithi // State variable (Sanskrit spelling)
krithiId // ID variable
KrithiEditor // Component name
handleDeitySave // Handler
handleTempleSave // Handler
onBack // Also a handler, different convention
className={`w-full h-12 px-4 border rounded-lg text-ink-900 focus:ring-2 focus:ring-primary focus:border-transparent transition-all ${highlight ? 'border-purple-400 bg-purple-50' : 'border-border-light bg-slate-50'}`}
Should use CSS modules or component variants.
| Metric | Current | Target |
|---|---|---|
| Lines of code | 1,860 | <400 per file |
| useState calls | 25+ | <10 (use reducer) |
any usages |
15+ | 0 |
| useEffect hooks | 5+ | Consolidated or custom hooks |
| Cyclomatic complexity | High | Medium |
| Test coverage | Unknown | >80% |
The component is functionally complete but suffers from classic React anti-patterns: monolithic design, type safety bypasses, complex state management, and mixed concerns. Refactoring into smaller, typed, testable components with proper state management would significantly improve maintainability and developer experience.
This review captured the pre-refactor state of KrithiEditor.tsx. Since then, TRACK-023 (Krithi Editor Refactoring) has implemented most of the high-priority items from krithi-editor-refactor-checklist.md:
MetadataTab, StructureTab, LyricsTab, TagsTab, AuditTab, NotationTab) and shared form primitives, reducing file size and improving separation of concerns.krithi-editor state, reducer, and supporting types (TabProps, KrithiEditorState, etc.) now replace the prior proliferation of useState and any, addressing the critical findings in sections 1–3 of this report.useKrithiData and useReferenceData, which now drive a bounded, predictable set of API calls and integrate with React Query for dashboard-level data.KrithiEditor.tsx).useKrithiData.ts (removing unstable toast dependencies) so the eager-load effect no longer loops.For a task-by-task implementation view, refer to krithi-editor-refactor-checklist.md, which tracks the detailed checklist items corresponding to the recommendations in this report.