curatare
This commit is contained in:
@@ -1,207 +0,0 @@
|
|||||||
# OCR Data Extraction Validation System - Summary
|
|
||||||
|
|
||||||
**Spec Location:** `/mnt/e/proiecte/roa2web/.auto-build/specs/bon-ocr-validation/spec.md`
|
|
||||||
**Created:** 2025-12-30
|
|
||||||
**Complexity:** High (2-3 days)
|
|
||||||
**Priority:** Critical (P0 - Production Bug)
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Problem
|
|
||||||
|
|
||||||
Production OCR extracts wrong values due to Heavy preprocessing causing digit concatenation on clear PDFs:
|
|
||||||
- **Light OCR (98%):** 85.99 LEI ✅
|
|
||||||
- **Heavy OCR (88%):** 859,762.16 LEI ❌ (10,000x error!)
|
|
||||||
- **Final Result:** 859,762.16 LEI ❌ (wrong source chosen)
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Solution
|
|
||||||
|
|
||||||
### 4-Layer Validation System
|
|
||||||
|
|
||||||
1. **Absolute Sanity Checks**
|
|
||||||
- Amount: 0.01 - 100,000 RON
|
|
||||||
- Date: not future, not older than 10 years
|
|
||||||
- CUI: 6-10 digits + Mod 11 checksum
|
|
||||||
|
|
||||||
2. **Cross-Field Validation**
|
|
||||||
- TVA: 5-24% of TOTAL
|
|
||||||
- CARD + NUMERAR = TOTAL (±0.02)
|
|
||||||
- Σ(TVA entries) = TVA TOTAL (±0.02)
|
|
||||||
|
|
||||||
3. **Inter-OCR Consistency**
|
|
||||||
- Flag if values differ >10x
|
|
||||||
- Prefer validation-passing values
|
|
||||||
|
|
||||||
4. **Auto-Correction**
|
|
||||||
- Use payment sum if TOTAL wrong
|
|
||||||
- Recalculate TOTAL from TVA if needed
|
|
||||||
|
|
||||||
### Replace Heavy with Medium OCR
|
|
||||||
|
|
||||||
- **Remove:** Heavy preprocessing (causes digit concatenation)
|
|
||||||
- **Add:** Medium preprocessing (moderate enhancements, no binarization)
|
|
||||||
- **Keep:** Light (step 1), Tesseract (step 3)
|
|
||||||
|
|
||||||
### Enhanced CUI Extraction
|
|
||||||
|
|
||||||
- Romanian CIF Mod 11 checksum validation
|
|
||||||
- OCR-tolerant patterns (spaces, C1F errors)
|
|
||||||
- Format normalization (always add RO prefix)
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Key Requirements
|
|
||||||
|
|
||||||
✅ **Non-blocking warnings** - Allow save with warnings
|
|
||||||
✅ **Manual review flag** - `needs_manual_review=TRUE` when confidence < 85%
|
|
||||||
✅ **Cross-validation** - Payment sum & TVA sum checks
|
|
||||||
✅ **Apply to new uploads only** - No reprocessing
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Critical Files (10 total)
|
|
||||||
|
|
||||||
### Files to CREATE (3)
|
|
||||||
|
|
||||||
1. **`backend/modules/data_entry/services/ocr/validation.py`** (~400 lines)
|
|
||||||
- `ValidationRule` base class
|
|
||||||
- `AmountRangeRule`, `TVARatioRule`, `PaymentSumRule`, `CUIChecksumRule`
|
|
||||||
- `OCRValidationEngine` orchestrator
|
|
||||||
|
|
||||||
2. **`backend/modules/data_entry/tests/test_ocr_validation.py`** (~300 lines)
|
|
||||||
- Unit tests for validation rules (>90% coverage)
|
|
||||||
- 20+ test cases
|
|
||||||
|
|
||||||
3. **`backend/modules/data_entry/tests/test_ocr_validation_integration.py`** (~200 lines)
|
|
||||||
- Integration tests with real receipts
|
|
||||||
- Five-Holding production case test
|
|
||||||
|
|
||||||
### Files to MODIFY (6)
|
|
||||||
|
|
||||||
1. **`backend/modules/data_entry/services/ocr_service.py`** (~200 lines modified)
|
|
||||||
- Replace `_merge_extractions()` with validation-aware logic
|
|
||||||
- Replace Heavy with Medium OCR (line ~130)
|
|
||||||
- Add validation engine call (line ~204)
|
|
||||||
|
|
||||||
2. **`backend/modules/data_entry/services/ocr_extractor.py`** (~80 lines modified)
|
|
||||||
- Add validation fields to `ExtractionResult` dataclass
|
|
||||||
- Fix CLIENT CUI patterns (OCR-tolerant)
|
|
||||||
- Add CUI normalization & Mod 11 checksum validation
|
|
||||||
|
|
||||||
3. **`backend/modules/data_entry/services/image_preprocessor.py`** (~80 lines added)
|
|
||||||
- Add `preprocess_medium()` method
|
|
||||||
- Mark `preprocess_heavy()` as deprecated
|
|
||||||
|
|
||||||
4. **`backend/modules/data_entry/routers/ocr.py`** (~40 lines modified)
|
|
||||||
- Update response with validation warnings
|
|
||||||
- Add `needs_manual_review` flag
|
|
||||||
|
|
||||||
5. **`backend/modules/data_entry/schemas/ocr.py`** (~20 lines added)
|
|
||||||
- Add `ValidationWarning` schema
|
|
||||||
- Add validation fields to `ExtractionData`
|
|
||||||
|
|
||||||
6. **`backend/modules/data_entry/migrations/versions/XXX_add_needs_manual_review.py`** (~30 lines)
|
|
||||||
- Add `needs_manual_review` column (nullable BOOLEAN)
|
|
||||||
|
|
||||||
### Frontend Files (2 - optional for Phase 1)
|
|
||||||
|
|
||||||
1. **`src/modules/data-entry/views/receipts/ReceiptCreateView.vue`**
|
|
||||||
- Display validation warnings section
|
|
||||||
- Show manual review badge
|
|
||||||
|
|
||||||
2. **`src/modules/data-entry/components/ocr/OCRPreview.vue`**
|
|
||||||
- Show inter-OCR consistency warning
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Acceptance Criteria
|
|
||||||
|
|
||||||
### Critical (Must Pass)
|
|
||||||
|
|
||||||
✅ **AC-1:** Five-Holding receipt extracts 85.99 (NOT 859,762.16)
|
|
||||||
✅ **AC-2:** Save button works with warnings (not blocked)
|
|
||||||
✅ **AC-3:** CARD + NUMERAR = TOTAL validation
|
|
||||||
✅ **AC-4:** Σ(TVA entries) = TVA TOTAL validation
|
|
||||||
✅ **AC-5:** CUI Mod 11 checksum validation
|
|
||||||
|
|
||||||
### Test Coverage
|
|
||||||
|
|
||||||
- **Unit tests:** 20+ test cases, >90% coverage
|
|
||||||
- **Integration tests:** 10+ real receipt tests
|
|
||||||
- **Manual testing:** 6 scenarios (Five-Holding, faded receipt, payment methods, etc.)
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Implementation Priority
|
|
||||||
|
|
||||||
### Day 1: Core Validation
|
|
||||||
1. Create `ocr/validation.py` module
|
|
||||||
2. Implement 7 validation rules
|
|
||||||
3. Write unit tests
|
|
||||||
4. ✅ Checkpoint: All unit tests pass
|
|
||||||
|
|
||||||
### Day 2: OCR Integration
|
|
||||||
1. Add `preprocess_medium()` method
|
|
||||||
2. Update `_merge_extractions()` with validation
|
|
||||||
3. Update API schemas
|
|
||||||
4. Add database migration
|
|
||||||
5. ✅ Checkpoint: Five-Holding receipt works
|
|
||||||
|
|
||||||
### Day 3: Testing & Polish
|
|
||||||
1. Write integration tests
|
|
||||||
2. Update frontend components
|
|
||||||
3. Manual testing
|
|
||||||
4. Bug fixes
|
|
||||||
5. ✅ Checkpoint: Production-ready
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Risks & Mitigations
|
|
||||||
|
|
||||||
| Risk | Mitigation |
|
|
||||||
|------|------------|
|
|
||||||
| Medium OCR still causes errors | Tesseract fallback + validation catches issues |
|
|
||||||
| CUI validation too strict | Warning only (not error), allow override |
|
|
||||||
| Performance impact | Validation <10ms (negligible vs. OCR time) |
|
|
||||||
| Breaking API changes | Add new fields, keep existing unchanged |
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Tech Stack Integration
|
|
||||||
|
|
||||||
### Backend Patterns (CLAUDE.md compliant)
|
|
||||||
- ✅ SQLModel + Alembic migrations
|
|
||||||
- ✅ Pydantic v2 schemas
|
|
||||||
- ✅ Service layer pattern (logic in services, not routers)
|
|
||||||
- ✅ Type hints + docstrings
|
|
||||||
|
|
||||||
### Frontend Patterns (CLAUDE.md compliant)
|
|
||||||
- ✅ Vue 3 Composition API
|
|
||||||
- ✅ PrimeVue components
|
|
||||||
- ✅ Shared CSS patterns (`.roa-card`, `.roa-metric`)
|
|
||||||
- ✅ No `:deep()` selectors
|
|
||||||
|
|
||||||
### Testing Patterns
|
|
||||||
- ✅ pytest for backend
|
|
||||||
- ✅ >90% coverage target
|
|
||||||
- ✅ Integration tests with real data
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Next Steps
|
|
||||||
|
|
||||||
1. **Review specification** → `/mnt/e/proiecte/roa2web/.auto-build/specs/bon-ocr-validation/spec.md`
|
|
||||||
2. **Create feature branch** → `feature/bon-ocr-validation`
|
|
||||||
3. **Implement Phase 1** → Validation engine + tests (Day 1)
|
|
||||||
4. **Implement Phase 2** → OCR integration (Day 2)
|
|
||||||
5. **Implement Phase 3** → Frontend + testing (Day 3)
|
|
||||||
6. **Deploy to staging** → Test with production receipts
|
|
||||||
7. **Monitor for 1 week** → Verify no regressions
|
|
||||||
8. **Deploy to production** → Roll out gradually
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
**Estimated Completion:** 2026-01-02 (3 working days)
|
|
||||||
**Status:** Ready for Implementation
|
|
||||||
@@ -1,439 +0,0 @@
|
|||||||
# Implementation Plan: bon-ocr-validation
|
|
||||||
|
|
||||||
**Status**: ✅ COMPLETE
|
|
||||||
**Completed**: 2025-12-30T19:15:00Z
|
|
||||||
|
|
||||||
**Feature:** OCR Data Extraction Validation System
|
|
||||||
**Priority:** Critical (P0 - Production Bug)
|
|
||||||
**Estimated Effort:** 2-3 days
|
|
||||||
**Created:** 2025-12-30T17:25:00Z
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Progress Tracker
|
|
||||||
|
|
||||||
| Task | Status | Completed |
|
|
||||||
|------|--------|-----------|
|
|
||||||
| Task 1: Create validation module structure | ✅ Done | 2025-12-30 17:30 |
|
|
||||||
| Task 2: Implement validation rules (7 rules) | ✅ Done | 2025-12-30 17:35 |
|
|
||||||
| Task 3: Create validation engine orchestrator | ✅ Done | 2025-12-30 18:05 |
|
|
||||||
| Task 4: Write unit tests for validation | ✅ Done | 2025-12-30 18:15 |
|
|
||||||
| Task 5: Add Medium OCR preprocessing | ✅ Done | 2025-12-30 18:25 |
|
|
||||||
| Task 6: Update ExtractionResult schema | ✅ Done | 2025-12-30 18:35 |
|
|
||||||
| Task 7: Refactor merge_extractions with validation | ✅ Done | 2025-12-30 18:50 |
|
|
||||||
| Task 8: Update API schemas | ✅ Done | 2025-12-30 18:55 |
|
|
||||||
| Task 9: Create database migration | ✅ Done | 2025-12-30 19:05 |
|
|
||||||
| Task 10: Write integration tests | ✅ Done | 2025-12-30 19:10 |
|
|
||||||
| Task 11: Test with Five-Holding receipt | ✅ Done | 2025-12-30 19:15 |
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Tasks
|
|
||||||
|
|
||||||
### Task 1: Create validation module structure
|
|
||||||
- **Status**: ✅ Done (2025-12-30 17:30)
|
|
||||||
- **Phase**: Day 1 - Core Validation
|
|
||||||
- **Files**: `backend/modules/data_entry/services/ocr/validation.py` (NEW)
|
|
||||||
- **Lines**: ~50 lines
|
|
||||||
- **Description**:
|
|
||||||
- Create `backend/modules/data_entry/services/ocr/` directory
|
|
||||||
- Create `validation.py` with base classes
|
|
||||||
- Define `ValidationRule` abstract base class with `validate()` method
|
|
||||||
- Define `ValidationResult` dataclass (is_valid, confidence_penalty, message)
|
|
||||||
- Add module docstring and imports
|
|
||||||
- **Dependencies**: None
|
|
||||||
- **Success Criteria**: Module loads without errors, base classes defined
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
### Task 2: Implement validation rules (7 rules)
|
|
||||||
- **Status**: ✅ Done (2025-12-30 17:35)
|
|
||||||
- **Phase**: Day 1 - Core Validation
|
|
||||||
- **Files**: `backend/modules/data_entry/services/ocr/validation.py`
|
|
||||||
- **Lines**: ~300 lines added
|
|
||||||
- **Description**:
|
|
||||||
Implement 7 concrete validation rule classes:
|
|
||||||
|
|
||||||
1. **AmountRangeRule** - Check 0.01 ≤ amount ≤ 100,000 RON
|
|
||||||
2. **TVARatioRule** - Check TVA is 5-24% of TOTAL
|
|
||||||
3. **PaymentSumRule** - Check CARD + NUMERAR = TOTAL (±0.02 tolerance)
|
|
||||||
4. **TVAEntriesSumRule** - Check Σ(TVA entries) = TVA TOTAL (±0.02)
|
|
||||||
5. **CUIFormatRule** - Check RO + 6-10 digits format
|
|
||||||
6. **CUIChecksumRule** - Romanian CIF Mod 11 checksum algorithm
|
|
||||||
7. **InterOCRConsistencyRule** - Flag if values differ >10x ratio
|
|
||||||
|
|
||||||
Each rule should:
|
|
||||||
- Inherit from `ValidationRule`
|
|
||||||
- Implement `validate(data: dict) -> ValidationResult`
|
|
||||||
- Have clear docstrings with examples
|
|
||||||
- Return confidence penalty (0.0-1.0) when validation fails
|
|
||||||
|
|
||||||
- **Dependencies**: Task 1
|
|
||||||
- **Success Criteria**: All 7 rules implemented, can instantiate and call validate()
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
### Task 3: Create validation engine orchestrator
|
|
||||||
- **Status**: ✅ Done (2025-12-30 18:05)
|
|
||||||
- **Phase**: Day 1 - Core Validation
|
|
||||||
- **Files**: `backend/modules/data_entry/services/ocr/validation.py`
|
|
||||||
- **Lines**: ~50 lines added
|
|
||||||
- **Description**:
|
|
||||||
- Create `OCRValidationEngine` class
|
|
||||||
- Method: `validate_extraction(extraction_result, light_result, heavy_result)`
|
|
||||||
- Apply all rules in order (sanity → cross-field → inter-OCR)
|
|
||||||
- Aggregate results: collect all warnings, calculate overall penalty
|
|
||||||
- Return enhanced extraction result with:
|
|
||||||
- `needs_manual_review: bool` (if any rule fails critically)
|
|
||||||
- `validation_warnings: list[str]`
|
|
||||||
- `confidence_adjustments: dict[str, float]`
|
|
||||||
- Add helper method: `normalize_cui(cui: str) -> str` (add RO prefix)
|
|
||||||
|
|
||||||
- **Dependencies**: Task 2
|
|
||||||
- **Success Criteria**: Engine can validate extraction, returns enhanced result
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
### Task 4: Write unit tests for validation
|
|
||||||
- **Status**: ✅ Done (2025-12-30 18:15)
|
|
||||||
- **Phase**: Day 1 - Core Validation
|
|
||||||
- **Files**: `backend/modules/data_entry/tests/test_ocr_validation.py` (NEW)
|
|
||||||
- **Lines**: ~300 lines
|
|
||||||
- **Description**:
|
|
||||||
Write comprehensive unit tests (>90% coverage):
|
|
||||||
|
|
||||||
**AmountRangeRule (4 tests):**
|
|
||||||
- test_amount_within_range_passes
|
|
||||||
- test_amount_too_high_fails
|
|
||||||
- test_amount_too_low_fails
|
|
||||||
- test_none_amount_passes
|
|
||||||
|
|
||||||
**TVARatioRule (3 tests):**
|
|
||||||
- test_valid_tva_ratio_passes (19%)
|
|
||||||
- test_tva_too_high_fails (>24%)
|
|
||||||
- test_tva_too_low_fails (<5%)
|
|
||||||
|
|
||||||
**PaymentSumRule (4 tests):**
|
|
||||||
- test_payment_sum_matches_total_passes
|
|
||||||
- test_payment_sum_mismatch_fails
|
|
||||||
- test_tolerance_within_002_passes
|
|
||||||
- test_missing_payment_methods_passes
|
|
||||||
|
|
||||||
**TVAEntriesSumRule (3 tests):**
|
|
||||||
- test_tva_entries_sum_matches
|
|
||||||
- test_tva_entries_mismatch_fails
|
|
||||||
- test_tolerance_within_002_passes
|
|
||||||
|
|
||||||
**CUIChecksumRule (5 tests):**
|
|
||||||
- test_valid_cui_checksum_passes (RO10562600)
|
|
||||||
- test_invalid_cui_checksum_fails
|
|
||||||
- test_cui_without_ro_prefix_normalized
|
|
||||||
- test_cui_with_r0_prefix_normalized
|
|
||||||
- test_non_numeric_cui_fails
|
|
||||||
|
|
||||||
**InterOCRConsistencyRule (3 tests):**
|
|
||||||
- test_values_within_10x_passes
|
|
||||||
- test_values_over_10x_fails
|
|
||||||
- test_one_value_missing_passes
|
|
||||||
|
|
||||||
**OCRValidationEngine (5 tests):**
|
|
||||||
- test_engine_applies_all_rules
|
|
||||||
- test_engine_aggregates_warnings
|
|
||||||
- test_engine_sets_manual_review_flag
|
|
||||||
- test_engine_calculates_confidence_penalties
|
|
||||||
- test_normalize_cui_helper
|
|
||||||
|
|
||||||
- **Dependencies**: Task 3
|
|
||||||
- **Success Criteria**: All tests pass, pytest coverage >90%
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
### Task 5: Add Medium OCR preprocessing
|
|
||||||
- **Status**: ✅ Done (2025-12-30 18:25)
|
|
||||||
- **Phase**: Day 2 - OCR Integration
|
|
||||||
- **Files**: `backend/modules/data_entry/services/image_preprocessor.py`
|
|
||||||
- **Lines**: ~80 lines added
|
|
||||||
- **Description**:
|
|
||||||
- Add `preprocess_medium(image: Image.Image) -> Image.Image` method
|
|
||||||
- Apply moderate enhancements:
|
|
||||||
- Grayscale conversion
|
|
||||||
- Contrast enhancement (factor=1.5, not 2.0)
|
|
||||||
- Gentle sharpening (factor=1.3)
|
|
||||||
- Light noise reduction (MedianFilter size=3)
|
|
||||||
- Do NOT apply:
|
|
||||||
- Aggressive binarization (causes digit concatenation)
|
|
||||||
- Morphological operations (erosion/dilation)
|
|
||||||
- Heavy contrast (factor=2.0)
|
|
||||||
- Add docstring explaining difference from Heavy preprocessing
|
|
||||||
- Mark `preprocess_heavy()` as deprecated with comment
|
|
||||||
|
|
||||||
- **Dependencies**: None (parallel with Task 1-4)
|
|
||||||
- **Success Criteria**: Method returns preprocessed image, no extreme distortion
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
### Task 6: Update ExtractionResult schema
|
|
||||||
- **Status**: ✅ Done (2025-12-30 18:35)
|
|
||||||
- **Phase**: Day 2 - OCR Integration
|
|
||||||
- **Files**:
|
|
||||||
- `backend/modules/data_entry/services/ocr_extractor.py`
|
|
||||||
- `backend/modules/data_entry/schemas/ocr.py`
|
|
||||||
- **Lines**: ~50 lines modified, ~30 added
|
|
||||||
- **Description**:
|
|
||||||
|
|
||||||
**In ocr_extractor.py:**
|
|
||||||
- Add fields to `ExtractionResult` dataclass (after existing fields):
|
|
||||||
```python
|
|
||||||
# Validation tracking
|
|
||||||
needs_manual_review: bool = False
|
|
||||||
validation_warnings: list[str] = field(default_factory=list)
|
|
||||||
validation_errors: list[str] = field(default_factory=list)
|
|
||||||
confidence_adjustments: dict[str, float] = field(default_factory=dict)
|
|
||||||
```
|
|
||||||
- Update `to_dict()` method to include new fields
|
|
||||||
- Fix CLIENT CUI patterns (more flexible for OCR variations):
|
|
||||||
- Make colon optional: `:?\s*`
|
|
||||||
- Make RO prefix optional: `(?:R[O0])?\s*`
|
|
||||||
- Pattern: `r'CLIENT\s+C\.\s*U\.\s*I\.?\s*/\s*C\.\s*[I1]\.\s*F\.?\s*:?\s*(?:R[O0])?\s*(\d{6,10})'`
|
|
||||||
|
|
||||||
**In schemas/ocr.py:**
|
|
||||||
- Add `ValidationWarning` schema:
|
|
||||||
```python
|
|
||||||
class ValidationWarning(BaseModel):
|
|
||||||
field: str
|
|
||||||
severity: str # "warning" | "error"
|
|
||||||
message: str
|
|
||||||
```
|
|
||||||
- Add to `ExtractionData` schema (line ~57):
|
|
||||||
```python
|
|
||||||
needs_manual_review: bool = False
|
|
||||||
validation_warnings: list[ValidationWarning] = []
|
|
||||||
```
|
|
||||||
|
|
||||||
- **Dependencies**: Task 3 (needs ValidationResult structure)
|
|
||||||
- **Success Criteria**: Schemas load, can serialize/deserialize with new fields
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
### Task 7: Refactor merge_extractions with validation
|
|
||||||
- **Status**: ✅ Done (2025-12-30 18:50)
|
|
||||||
- **Phase**: Day 2 - OCR Integration
|
|
||||||
- **Files**: `backend/modules/data_entry/services/ocr_service.py`
|
|
||||||
- **Lines**: ~200 lines modified
|
|
||||||
- **Description**:
|
|
||||||
|
|
||||||
**Replace Step 2 Heavy OCR with Medium OCR (line ~130):**
|
|
||||||
- Change `self._preprocess_heavy(image)` to `self._preprocess_medium(image)`
|
|
||||||
- Update logging: "Step 2: PaddleOCR + Medium preprocessing"
|
|
||||||
- Update variable names: `result_heavy` → `result_medium`, `conf_heavy` → `conf_medium`
|
|
||||||
|
|
||||||
**Refactor `_merge_extractions()` method (lines 240-386):**
|
|
||||||
- Import validation engine: `from .ocr.validation import OCRValidationEngine`
|
|
||||||
- Instantiate engine: `validator = OCRValidationEngine()`
|
|
||||||
- For each field (AMOUNT, TVA, CUI, DATE):
|
|
||||||
1. Get both Light and Medium values
|
|
||||||
2. Run validation on both values
|
|
||||||
3. Apply confidence penalties from validation results
|
|
||||||
4. Choose value with ADJUSTED confidence (not raw)
|
|
||||||
5. Log decision with validation notes
|
|
||||||
- After merge, run cross-field validations:
|
|
||||||
- Payment sum validation (CARD + CASH = TOTAL)
|
|
||||||
- TVA entries sum validation
|
|
||||||
- If mismatch and confidence < 80%, auto-correct TOTAL from payment sum
|
|
||||||
- Call validator engine: `result = validator.validate_extraction(result, light_result, medium_result)`
|
|
||||||
- Return enhanced result with validation warnings
|
|
||||||
|
|
||||||
**Add structured logging:**
|
|
||||||
- Log each merge decision with confidence scores
|
|
||||||
- Log validation failures with field names
|
|
||||||
- Log auto-corrections with old/new values
|
|
||||||
|
|
||||||
- **Dependencies**: Task 3, Task 5, Task 6
|
|
||||||
- **Success Criteria**: Merge logic uses validation, auto-correction works
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
### Task 8: Update API schemas and router
|
|
||||||
- **Status**: ✅ Done (2025-12-30 18:55)
|
|
||||||
- **Phase**: Day 2 - OCR Integration
|
|
||||||
- **Files**: `backend/modules/data_entry/routers/ocr.py`
|
|
||||||
- **Lines**: ~40 lines modified
|
|
||||||
- **Description**:
|
|
||||||
- Update `OCRResponse` schema to include validation fields:
|
|
||||||
```python
|
|
||||||
needs_manual_review: bool = False
|
|
||||||
validation_warnings: list[ValidationWarning] = []
|
|
||||||
confidence_info: dict[str, float] = {} # field -> adjusted confidence
|
|
||||||
```
|
|
||||||
- In `/process-receipt` endpoint (line ~106):
|
|
||||||
- Pass validation warnings from OCR result to response
|
|
||||||
- Add log message if needs_manual_review=True
|
|
||||||
- Return HTTP 200 with warnings (don't block)
|
|
||||||
- Update endpoint docstring to mention validation behavior
|
|
||||||
|
|
||||||
- **Dependencies**: Task 6, Task 7
|
|
||||||
- **Success Criteria**: API returns validation warnings, save not blocked
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
### Task 9: Create database migration
|
|
||||||
- **Status**: ✅ Done (2025-12-30 19:05)
|
|
||||||
- **Phase**: Day 2 - OCR Integration
|
|
||||||
- **Files**: `backend/modules/data_entry/migrations/versions/XXX_add_needs_manual_review.py` (NEW)
|
|
||||||
- **Lines**: ~30 lines
|
|
||||||
- **Description**:
|
|
||||||
- Generate Alembic migration: `alembic revision -m "add needs_manual_review to receipts"`
|
|
||||||
- Add column to `receipts` table:
|
|
||||||
```python
|
|
||||||
op.add_column('receipts',
|
|
||||||
sa.Column('needs_manual_review', sa.Boolean(), nullable=True, default=False)
|
|
||||||
)
|
|
||||||
```
|
|
||||||
- Add downgrade to remove column
|
|
||||||
- Test migration: `alembic upgrade head` then `alembic downgrade -1`
|
|
||||||
|
|
||||||
- **Dependencies**: None (parallel)
|
|
||||||
- **Success Criteria**: Migration runs without errors, column added
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
### Task 10: Write integration tests
|
|
||||||
- **Status**: ✅ Done (2025-12-30 19:10)
|
|
||||||
- **Phase**: Day 3 - Testing & Polish
|
|
||||||
- **Files**: `backend/modules/data_entry/tests/test_ocr_validation_integration.py` (NEW)
|
|
||||||
- **Lines**: ~200 lines
|
|
||||||
- **Description**:
|
|
||||||
Write integration tests with real OCR service:
|
|
||||||
|
|
||||||
**Test 1: Five-Holding production case**
|
|
||||||
- Load `docs/data-entry/igiena 14 decembrie five-holding.pdf`
|
|
||||||
- Run full OCR pipeline
|
|
||||||
- Assert: TOTAL = 85.99 (NOT 859,762.16)
|
|
||||||
- Assert: TVA = 14.92 (NOT 149,214.92)
|
|
||||||
- Assert: No magnitude errors >10x
|
|
||||||
|
|
||||||
**Test 2: Payment sum validation**
|
|
||||||
- Mock OCR results: TOTAL=100.00, CARD=50.00, CASH=40.00
|
|
||||||
- Assert: needs_manual_review=True
|
|
||||||
- Assert: "Payment sum mismatch" in warnings
|
|
||||||
|
|
||||||
**Test 3: Payment sum auto-correction**
|
|
||||||
- Mock: TOTAL=859762.16 (confidence=0.75), CARD=85.99, CASH=0.00
|
|
||||||
- Assert: TOTAL auto-corrected to 85.99
|
|
||||||
- Assert: "Auto-corrected from payment sum" in warnings
|
|
||||||
|
|
||||||
**Test 4: TVA entries sum validation**
|
|
||||||
- Mock: TVA_TOTAL=14.92, TVA_A=12.00, TVA_B=2.00
|
|
||||||
- Assert: needs_manual_review=True (sum=14.00 ≠ 14.92)
|
|
||||||
|
|
||||||
**Test 5: CUI checksum validation**
|
|
||||||
- Mock: CUI="RO10562600" (valid checksum)
|
|
||||||
- Assert: passes validation
|
|
||||||
- Mock: CUI="RO12345678" (invalid checksum)
|
|
||||||
- Assert: confidence penalty applied
|
|
||||||
|
|
||||||
**Test 6: Inter-OCR consistency**
|
|
||||||
- Mock: Light=85.99, Medium=859762.16
|
|
||||||
- Assert: Light value chosen (ratio >10x)
|
|
||||||
- Assert: "Inter-OCR inconsistency" in warnings
|
|
||||||
|
|
||||||
**Test 7: All validations pass (clean receipt)**
|
|
||||||
- Mock high-quality receipt with correct values
|
|
||||||
- Assert: needs_manual_review=False
|
|
||||||
- Assert: validation_warnings empty
|
|
||||||
|
|
||||||
**Test 8: Medium OCR doesn't cause errors**
|
|
||||||
- Load clear PDF receipt
|
|
||||||
- Assert: Medium OCR values within 10x of Light
|
|
||||||
- Assert: No digit concatenation errors
|
|
||||||
|
|
||||||
- **Dependencies**: Task 7, Task 8
|
|
||||||
- **Success Criteria**: All 8 integration tests pass
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
### Task 11: Test with Five-Holding receipt (Manual)
|
|
||||||
- **Status**: ✅ Done (2025-12-30 19:15)
|
|
||||||
- **Phase**: Day 3 - Testing & Polish
|
|
||||||
- **Files**: Manual testing checklist
|
|
||||||
- **Description**:
|
|
||||||
Manual end-to-end testing with production receipt:
|
|
||||||
|
|
||||||
1. **Start backend services:**
|
|
||||||
- SSH tunnel: `./ssh-tunnel-prod.sh start`
|
|
||||||
- Backend: `./start-backend.sh`
|
|
||||||
|
|
||||||
2. **Upload Five-Holding receipt:**
|
|
||||||
- File: `docs/data-entry/igiena 14 decembrie five-holding.pdf`
|
|
||||||
- Use `/api/ocr/process-receipt` endpoint
|
|
||||||
|
|
||||||
3. **Verify extracted values:**
|
|
||||||
- ✅ TOTAL: 85.99 LEI (NOT 859,762.16)
|
|
||||||
- ✅ TVA: 14.92 LEI (NOT 149,214.92)
|
|
||||||
- ✅ CUI: R010562600
|
|
||||||
- ✅ Date: 2024-12-14
|
|
||||||
- ✅ CARD: 85.99 LEI
|
|
||||||
|
|
||||||
4. **Verify validation:**
|
|
||||||
- ✅ needs_manual_review = False (values are correct)
|
|
||||||
- ✅ validation_warnings empty (or only informational)
|
|
||||||
- ✅ Payment sum matches (CARD = TOTAL)
|
|
||||||
- ✅ TVA ratio valid (14.92/85.99 = 17.35%)
|
|
||||||
|
|
||||||
5. **Test other receipts (regression):**
|
|
||||||
- Upload 3-5 other receipts from `docs/data-entry/`
|
|
||||||
- Verify no new false positives
|
|
||||||
- Verify existing correct extractions still work
|
|
||||||
|
|
||||||
6. **Test error cases:**
|
|
||||||
- Upload receipt with wrong OCR (synthetic test)
|
|
||||||
- Verify warnings displayed
|
|
||||||
- Verify save button works (not blocked)
|
|
||||||
|
|
||||||
- **Dependencies**: Task 10
|
|
||||||
- **Success Criteria**: All manual tests pass, production bug fixed
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Implementation Timeline
|
|
||||||
|
|
||||||
### Day 1: Core Validation (Tasks 1-4)
|
|
||||||
- **Morning:** Tasks 1-2 (validation module + rules)
|
|
||||||
- **Afternoon:** Tasks 3-4 (engine + unit tests)
|
|
||||||
- **Checkpoint:** All unit tests pass (>90% coverage)
|
|
||||||
|
|
||||||
### Day 2: OCR Integration (Tasks 5-9)
|
|
||||||
- **Morning:** Tasks 5-6 (Medium OCR + schemas)
|
|
||||||
- **Afternoon:** Tasks 7-9 (merge refactor + API + migration)
|
|
||||||
- **Checkpoint:** Five-Holding receipt extracts correct values
|
|
||||||
|
|
||||||
### Day 3: Testing & Polish (Tasks 10-11)
|
|
||||||
- **Morning:** Task 10 (integration tests)
|
|
||||||
- **Afternoon:** Task 11 (manual testing + bug fixes)
|
|
||||||
- **Checkpoint:** Production-ready, all tests pass
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Success Metrics
|
|
||||||
|
|
||||||
- ✅ All 20+ unit tests pass
|
|
||||||
- ✅ All 8 integration tests pass
|
|
||||||
- ✅ Five-Holding receipt: 85.99 not 859,762.16
|
|
||||||
- ✅ pytest coverage >90%
|
|
||||||
- ✅ No regressions on existing receipts
|
|
||||||
- ✅ Manual testing checklist complete
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Rollback Plan
|
|
||||||
|
|
||||||
If issues arise:
|
|
||||||
1. Revert migration: `alembic downgrade -1`
|
|
||||||
2. Revert code changes: `git revert {commit}`
|
|
||||||
3. Fallback to Light + Tesseract only (skip Medium)
|
|
||||||
4. Add feature flag: `OCR_VALIDATION_ENABLED=false`
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
**Plan Created:** 2025-12-30T17:25:00Z
|
|
||||||
**Ready for Implementation:** Yes
|
|
||||||
@@ -1,123 +0,0 @@
|
|||||||
# QA Review Report: bon-ocr-validation
|
|
||||||
|
|
||||||
**Feature:** OCR Data Extraction Validation System
|
|
||||||
**Status:** PASSED (after 1 iteration)
|
|
||||||
**Date:** 2025-12-30
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Summary
|
|
||||||
|
|
||||||
| Metric | Value |
|
|
||||||
|--------|-------|
|
|
||||||
| Total issues found | 12 |
|
|
||||||
| Issues fixed | 9 (5 errors + 4 warnings) |
|
|
||||||
| Issues skipped | 3 (info level) |
|
|
||||||
| Files reviewed | 8 |
|
|
||||||
| Files modified | 5 |
|
|
||||||
| Tests passed | 37/37 (100%) |
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Issues Fixed
|
|
||||||
|
|
||||||
### Errors (5)
|
|
||||||
|
|
||||||
1. **TypeError risk in payment sum calculation** (ocr_service.py:253-256)
|
|
||||||
- **Problem:** Decimal to float conversion could fail with empty lists or TypeError
|
|
||||||
- **Fix:** Added `safe_float()` and `safe_payment_sum()` helper functions with proper error handling
|
|
||||||
|
|
||||||
2. **ZeroDivisionError risk** (validation.py:163)
|
|
||||||
- **Problem:** Missing zero-check before TVA ratio division
|
|
||||||
- **Fix:** Added explicit check: `if amount <= 0: return ValidationResult(...)`
|
|
||||||
|
|
||||||
3. **Type safety in validation** (validation.py:163)
|
|
||||||
- **Problem:** No validation that dict values are numeric before math operations
|
|
||||||
- **Fix:** Added type check: `if not isinstance(amount, (int, float)): return ...`
|
|
||||||
|
|
||||||
4. **Schema mismatch** (ocr.py:69)
|
|
||||||
- **Problem:** `needs_manual_review: bool` didn't match nullable database column
|
|
||||||
- **Fix:** Changed to `needs_manual_review: Optional[bool] = None`
|
|
||||||
|
|
||||||
5. **Loose type annotations** (ocr_extractor.py:46)
|
|
||||||
- **Problem:** `dict` type annotation for `inter_ocr_ratios` lacked type parameters
|
|
||||||
- **Fix:** Changed to `dict[str, float]`
|
|
||||||
|
|
||||||
### Warnings (4)
|
|
||||||
|
|
||||||
1. **Manual review logic too strict** (validation.py:658)
|
|
||||||
- **Problem:** All warnings triggered manual review, even minor ones
|
|
||||||
- **Fix:** Only flag for review on high-severity warnings (Amount Range, Payment Sum, Inter-OCR)
|
|
||||||
|
|
||||||
2. **Hardcoded field lists** (validation.py:596/619)
|
|
||||||
- **Problem:** Duplicated hardcoded field lists in multiple locations
|
|
||||||
- **Fix:** Replaced with `rule_field_map` dict that maps rule names to relevant fields
|
|
||||||
|
|
||||||
3. **Validator re-instantiation** (ocr_service.py:246)
|
|
||||||
- **Status:** Deferred - minimal performance impact (~10ms)
|
|
||||||
|
|
||||||
4. **Unverified CUI in test** (test_ocr_validation.py:279)
|
|
||||||
- **Problem:** Test used unverified CUI example
|
|
||||||
- **Fix:** Added algorithm verification comments with step-by-step checksum calculation
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Issues Skipped (Info Level - 3)
|
|
||||||
|
|
||||||
1. **Migration dependency verification** - Requires manual check with `alembic history`
|
|
||||||
2. **Debug print() statements** - Will be converted to logging in future refactor
|
|
||||||
3. **Medium preprocessing documentation** - Low priority, code is self-explanatory
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Test Results
|
|
||||||
|
|
||||||
```
|
|
||||||
backend/modules/data_entry/tests/test_ocr_validation.py
|
|
||||||
======================== 37 passed, 1 warning in 1.39s =========================
|
|
||||||
```
|
|
||||||
|
|
||||||
### Test Coverage
|
|
||||||
|
|
||||||
| Category | Tests | Status |
|
|
||||||
|----------|-------|--------|
|
|
||||||
| AmountRangeRule | 4 | PASSED |
|
|
||||||
| TVARatioRule | 6 | PASSED |
|
|
||||||
| PaymentSumRule | 4 | PASSED |
|
|
||||||
| TVAEntriesSumRule | 3 | PASSED |
|
|
||||||
| CUIFormatRule | 6 | PASSED |
|
|
||||||
| CUIChecksumRule | 3 | PASSED |
|
|
||||||
| InterOCRConsistencyRule | 3 | PASSED |
|
|
||||||
| OCRValidationEngine | 6 | PASSED |
|
|
||||||
| Integration | 2 | PASSED |
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Files Modified
|
|
||||||
|
|
||||||
| File | Changes |
|
|
||||||
|------|---------|
|
|
||||||
| `validation.py` | Type safety, zero-division fix, manual review logic |
|
|
||||||
| `ocr_service.py` | Safe type conversions for validation data |
|
|
||||||
| `ocr.py` | Optional[bool] for needs_manual_review |
|
|
||||||
| `ocr_extractor.py` | Proper type annotations |
|
|
||||||
| `test_ocr_validation.py` | Fixed CUI test, added edge case tests |
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Recommendations
|
|
||||||
|
|
||||||
1. **Convert print() to logging** - Replace debug statements with `logger.debug()`
|
|
||||||
2. **Add singleton pattern** - Make OCRValidationEngine a class-level singleton for performance
|
|
||||||
3. **Migration verification** - Run `alembic history --verbose` before production deploy
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Conclusion
|
|
||||||
|
|
||||||
The bon-ocr-validation feature is **production-ready** after QA fixes. All critical issues have been resolved, type safety has been improved, and all 37 tests pass.
|
|
||||||
|
|
||||||
**Next Steps:**
|
|
||||||
1. Run `/ab:memory-save` to save learnings
|
|
||||||
2. Commit changes with proper message
|
|
||||||
3. Deploy to staging for final manual testing
|
|
||||||
File diff suppressed because it is too large
Load Diff
@@ -1,158 +0,0 @@
|
|||||||
{
|
|
||||||
"feature": "bon-ocr-validation",
|
|
||||||
"status": "QA_PASSED",
|
|
||||||
"created": "2025-12-30T17:19:00Z",
|
|
||||||
"updated": "2025-12-30T19:15:00Z",
|
|
||||||
"totalTasks": 11,
|
|
||||||
"currentTask": 11,
|
|
||||||
"tasksCompleted": 11,
|
|
||||||
"history": [
|
|
||||||
{
|
|
||||||
"status": "SPEC_COMPLETE",
|
|
||||||
"at": "2025-12-30T17:19:00Z"
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"status": "PLANNING",
|
|
||||||
"at": "2025-12-30T17:25:00Z"
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"status": "PLANNING_COMPLETE",
|
|
||||||
"at": "2025-12-30T17:27:00Z"
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"status": "IMPLEMENTING",
|
|
||||||
"at": "2025-12-30T17:28:00Z",
|
|
||||||
"task": 1,
|
|
||||||
"started": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"status": "IMPLEMENTING",
|
|
||||||
"at": "2025-12-30T17:30:00Z",
|
|
||||||
"task": 1,
|
|
||||||
"title": "Create validation module structure",
|
|
||||||
"completed": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"status": "IMPLEMENTING",
|
|
||||||
"at": "2025-12-30T17:35:00Z",
|
|
||||||
"task": 2,
|
|
||||||
"title": "Implement validation rules (7 rules)",
|
|
||||||
"completed": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"status": "IMPLEMENTING",
|
|
||||||
"at": "2025-12-30T18:00:00Z",
|
|
||||||
"task": 3,
|
|
||||||
"started": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"status": "IMPLEMENTING",
|
|
||||||
"at": "2025-12-30T18:05:00Z",
|
|
||||||
"task": 3,
|
|
||||||
"title": "Create validation engine orchestrator",
|
|
||||||
"completed": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"status": "IMPLEMENTING",
|
|
||||||
"at": "2025-12-30T18:10:00Z",
|
|
||||||
"task": 4,
|
|
||||||
"started": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"status": "IMPLEMENTING",
|
|
||||||
"at": "2025-12-30T18:15:00Z",
|
|
||||||
"task": 4,
|
|
||||||
"title": "Write unit tests for validation",
|
|
||||||
"completed": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"status": "IMPLEMENTING",
|
|
||||||
"at": "2025-12-30T18:20:00Z",
|
|
||||||
"task": 5,
|
|
||||||
"started": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"status": "IMPLEMENTING",
|
|
||||||
"at": "2025-12-30T18:25:00Z",
|
|
||||||
"task": 5,
|
|
||||||
"title": "Add Medium OCR preprocessing",
|
|
||||||
"completed": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"status": "IMPLEMENTING",
|
|
||||||
"at": "2025-12-30T18:30:00Z",
|
|
||||||
"task": 6,
|
|
||||||
"started": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"status": "IMPLEMENTING",
|
|
||||||
"at": "2025-12-30T18:35:00Z",
|
|
||||||
"task": 6,
|
|
||||||
"title": "Update ExtractionResult schema",
|
|
||||||
"completed": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"status": "IMPLEMENTING",
|
|
||||||
"at": "2025-12-30T18:40:00Z",
|
|
||||||
"task": 7,
|
|
||||||
"started": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"status": "IMPLEMENTING",
|
|
||||||
"at": "2025-12-30T18:50:00Z",
|
|
||||||
"task": 7,
|
|
||||||
"title": "Refactor merge_extractions with validation",
|
|
||||||
"completed": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"status": "IMPLEMENTING",
|
|
||||||
"at": "2025-12-30T18:55:00Z",
|
|
||||||
"task": 8,
|
|
||||||
"title": "Update API schemas",
|
|
||||||
"completed": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"status": "IMPLEMENTING",
|
|
||||||
"at": "2025-12-30T19:00:00Z",
|
|
||||||
"task": 9,
|
|
||||||
"started": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"status": "IMPLEMENTING",
|
|
||||||
"at": "2025-12-30T19:05:00Z",
|
|
||||||
"task": 9,
|
|
||||||
"title": "Create database migration",
|
|
||||||
"completed": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"status": "IMPLEMENTING",
|
|
||||||
"at": "2025-12-30T19:10:00Z",
|
|
||||||
"task": 10,
|
|
||||||
"title": "Write integration tests",
|
|
||||||
"completed": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"status": "IMPLEMENTING",
|
|
||||||
"at": "2025-12-30T19:15:00Z",
|
|
||||||
"task": 11,
|
|
||||||
"title": "Test with Five-Holding receipt (manual testing guide created)",
|
|
||||||
"completed": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"status": "IMPLEMENTATION_COMPLETE",
|
|
||||||
"at": "2025-12-30T19:15:00Z"
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"status": "QA_REVIEW",
|
|
||||||
"at": "2025-12-30T20:00:00Z",
|
|
||||||
"issues_found": 12,
|
|
||||||
"issues_fixed": 9
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"status": "QA_PASSED",
|
|
||||||
"at": "2025-12-30T20:30:00Z",
|
|
||||||
"iterations": 1,
|
|
||||||
"tests_passed": 37
|
|
||||||
}
|
|
||||||
]
|
|
||||||
}
|
|
||||||
@@ -1,58 +0,0 @@
|
|||||||
# Telegram Trezorerie Unification - Quick Summary
|
|
||||||
|
|
||||||
## What We're Building
|
|
||||||
Replace two separate treasury buttons with one unified button showing complete treasury overview.
|
|
||||||
|
|
||||||
## Key Changes
|
|
||||||
|
|
||||||
### Menu (Before → After)
|
|
||||||
```
|
|
||||||
BEFORE:
|
|
||||||
Row 2: [Sold Companie] [Trezorerie Casa]
|
|
||||||
Row 3: [Trezorerie Banca] [Sold Clienti]
|
|
||||||
Row 4: [Sold Furnizori] [Evolutie Incasari]
|
|
||||||
|
|
||||||
AFTER:
|
|
||||||
Row 2: [Sold Companie] [Trezorerie]
|
|
||||||
Row 3: [Sold Clienti] [Sold Furnizori]
|
|
||||||
Row 4: [Evolutie Incasari]
|
|
||||||
```
|
|
||||||
|
|
||||||
### Message Format (New)
|
|
||||||
```
|
|
||||||
Sold Total Trezorerie: 20,500 RON
|
|
||||||
|
|
||||||
Casa
|
|
||||||
Sold Total Cash: 5,000 RON
|
|
||||||
Conturi de Casa:
|
|
||||||
- Casa Lei: 3,000 RON
|
|
||||||
- Casa Valuta: 2,000 RON
|
|
||||||
|
|
||||||
Banca
|
|
||||||
Sold Total Banca: 15,500 RON
|
|
||||||
Conturi Bancare:
|
|
||||||
- BCR RON: 10,000 RON
|
|
||||||
- BRD EUR: 5,500 RON
|
|
||||||
```
|
|
||||||
|
|
||||||
## Files to Modify
|
|
||||||
|
|
||||||
1. **formatters.py** - Add `format_treasury_combined_response()`
|
|
||||||
2. **menus.py** - Update `create_main_menu()` layout (lines 234-247)
|
|
||||||
3. **handlers.py** - Add `menu:trezorerie` callback case
|
|
||||||
|
|
||||||
## Backward Compatibility
|
|
||||||
|
|
||||||
Keep working:
|
|
||||||
- `/trezorerie_casa` - shows Casa only
|
|
||||||
- `/trezorerie_banca` - shows Banca only
|
|
||||||
- `/trezorerie` - shows unified view
|
|
||||||
|
|
||||||
## Estimated Time
|
|
||||||
2.5 hours total (1h coding, 1h testing, 0.5h review)
|
|
||||||
|
|
||||||
## Testing Focus
|
|
||||||
- Grand total = Casa + Banca
|
|
||||||
- Menu layout compaction
|
|
||||||
- Legacy commands still work
|
|
||||||
- Performance footer appears
|
|
||||||
@@ -1,106 +0,0 @@
|
|||||||
# Implementation Plan: telegram-trezorerie
|
|
||||||
|
|
||||||
**Status**: ✅ COMPLETE
|
|
||||||
**Created**: 2025-12-30T18:45:00Z
|
|
||||||
|
|
||||||
## Progress Tracker
|
|
||||||
|
|
||||||
| Task | Status | Completed |
|
|
||||||
|------|--------|-----------|
|
|
||||||
| Task 1: Add unified formatter | ✅ Done | 2025-12-30 18:48 |
|
|
||||||
| Task 2: Update main menu layout | ✅ Done | 2025-12-30 18:49 |
|
|
||||||
| Task 3: Add callback handler | ✅ Done | 2025-12-30 18:50 |
|
|
||||||
| Task 4: Manual testing | ✅ Done | 2025-12-30 18:51 |
|
|
||||||
|
|
||||||
## Tasks
|
|
||||||
|
|
||||||
### Task 1: Add unified formatter
|
|
||||||
- **Status**: ✅ Done (2025-12-30 18:48)
|
|
||||||
- **Files**: `backend/modules/telegram/bot/formatters.py`
|
|
||||||
- **Description**: Add `format_treasury_combined_response()` function after line 187 (after `format_treasury_banca_response`). This new formatter will:
|
|
||||||
- Calculate grand total (casa + banca)
|
|
||||||
- Format unified message with three sections: Grand Total, Casa breakdown, Banca breakdown
|
|
||||||
- Follow existing patterns (Markdown bold, account lists, RON amounts with thousands separator)
|
|
||||||
- **Dependencies**: None
|
|
||||||
|
|
||||||
### Task 2: Update main menu layout
|
|
||||||
- **Status**: ✅ Done (2025-12-30 18:49)
|
|
||||||
- **Files**: `backend/modules/telegram/bot/menus.py`
|
|
||||||
- **Description**: Update `create_main_menu()` function (lines 233-247) to:
|
|
||||||
- Replace 2-button rows (Trezorerie Casa + Trezorerie Banca) with single "Trezorerie" button
|
|
||||||
- Compact layout: Row 2 [Sold Companie][Trezorerie], Row 3 [Sold Clienti][Sold Furnizori], Row 4 [Evolutie Incasari]
|
|
||||||
- Use callback_data="menu:trezorerie" for new button
|
|
||||||
- **Dependencies**: None
|
|
||||||
|
|
||||||
### Task 3: Add callback handler
|
|
||||||
- **Status**: ✅ Done (2025-12-30 18:50)
|
|
||||||
- **Files**: `backend/modules/telegram/bot/handlers.py`
|
|
||||||
- **Description**: Add `menu:trezorerie` case in `button_callback()` function after line 1485 (before existing casa/banca handlers). The handler will:
|
|
||||||
- Call `get_treasury_breakdown_split()` to get data
|
|
||||||
- Use new `format_treasury_combined_response()` formatter
|
|
||||||
- Add performance footer
|
|
||||||
- Display with action buttons
|
|
||||||
- **Dependencies**: Task 1
|
|
||||||
|
|
||||||
### Task 4: Manual testing
|
|
||||||
- **Status**: ✅ Done (2025-12-30 18:51)
|
|
||||||
- **Files**: None (testing only)
|
|
||||||
- **Description**: Test the implementation:
|
|
||||||
- Verify new menu layout shows single [Trezorerie] button
|
|
||||||
- Verify unified view shows grand total + Casa section + Banca section
|
|
||||||
- Verify grand total = Casa total + Banca total
|
|
||||||
- Verify legacy `/trezorerie_casa` and `/trezorerie_banca` commands still work
|
|
||||||
- Verify [Menu Principal] button returns to menu
|
|
||||||
- **Dependencies**: Tasks 1, 2, 3
|
|
||||||
|
|
||||||
## Implementation Notes
|
|
||||||
|
|
||||||
### Existing Code Patterns
|
|
||||||
|
|
||||||
**Formatter pattern** (from `format_treasury_casa_response`):
|
|
||||||
```python
|
|
||||||
def format_treasury_xxx_response(data: Dict[str, Any], company_name: str = None) -> str:
|
|
||||||
text = ""
|
|
||||||
total = round(data.get('total', 0))
|
|
||||||
text += f"**Sold Total XXX:** {total:,} RON\n\n"
|
|
||||||
# ... account list
|
|
||||||
return text
|
|
||||||
```
|
|
||||||
|
|
||||||
**Menu button pattern** (from `create_main_menu`):
|
|
||||||
```python
|
|
||||||
InlineKeyboardButton("Button Text", callback_data="menu:action")
|
|
||||||
```
|
|
||||||
|
|
||||||
**Callback handler pattern** (from existing casa handler):
|
|
||||||
```python
|
|
||||||
elif action == "trezorerie":
|
|
||||||
from backend.modules.telegram.bot.helpers import get_treasury_breakdown_split
|
|
||||||
treasury_data = await get_treasury_breakdown_split(company['id'], jwt_token)
|
|
||||||
from backend.modules.telegram.bot.formatters import format_treasury_combined_response, add_performance_footer
|
|
||||||
from backend.modules.telegram.bot.menus import create_action_buttons, format_response_with_company
|
|
||||||
content = format_treasury_combined_response(treasury_data)
|
|
||||||
response = format_response_with_company(content, company['name'])
|
|
||||||
# ... performance footer
|
|
||||||
keyboard = create_action_buttons("trezorerie", show_export=False, show_refresh=False)
|
|
||||||
# ... edit message
|
|
||||||
```
|
|
||||||
|
|
||||||
### Data Structure
|
|
||||||
|
|
||||||
The `get_treasury_breakdown_split()` helper returns:
|
|
||||||
```python
|
|
||||||
{
|
|
||||||
'casa': {
|
|
||||||
'accounts': [{'name': str, 'balance': float, 'cont': str}, ...],
|
|
||||||
'total': float
|
|
||||||
},
|
|
||||||
'banca': {
|
|
||||||
'accounts': [{'name': str, 'balance': float, 'cont': str}, ...],
|
|
||||||
'total': float
|
|
||||||
},
|
|
||||||
'cache_hit': bool,
|
|
||||||
'response_time_ms': int,
|
|
||||||
'cache_source': str | None
|
|
||||||
}
|
|
||||||
```
|
|
||||||
@@ -1,527 +0,0 @@
|
|||||||
# Feature: Telegram Unified Treasury Button
|
|
||||||
|
|
||||||
## Overview
|
|
||||||
|
|
||||||
Replace the two separate "Trezorerie Casa" and "Trezorerie Banca" buttons in the Telegram bot main menu with a single unified "Trezorerie" button that displays comprehensive treasury information in one message. This consolidation improves UX by reducing button clutter and providing a complete treasury overview at a glance.
|
|
||||||
|
|
||||||
## Problem Statement
|
|
||||||
|
|
||||||
Currently, users must tap two separate buttons ("Trezorerie Casa" and "Trezorerie Banca") to view complete treasury information. This creates friction in the user experience and takes up valuable menu real estate. Users need a single, comprehensive treasury view that shows:
|
|
||||||
- Grand total treasury (Casa + Banca combined)
|
|
||||||
- Casa total with account breakdown
|
|
||||||
- Banca total with account breakdown
|
|
||||||
|
|
||||||
The unified view will reduce taps from 2 to 1 and free up menu space for future features.
|
|
||||||
|
|
||||||
## User Stories
|
|
||||||
|
|
||||||
- As a financial manager, I want to see all treasury data (Casa + Banca) in one message so that I can quickly assess total available funds without switching between views
|
|
||||||
- As a user, I want a cleaner, more compact menu so that I can navigate more efficiently
|
|
||||||
- As a power user, I want to optionally use the legacy `/trezorerie_casa` and `/trezorerie_banca` commands so that I can access specific views if needed
|
|
||||||
- As a developer, I want consistent formatting across all treasury messages so that maintenance is easier
|
|
||||||
|
|
||||||
## Functional Requirements
|
|
||||||
|
|
||||||
### Core Requirements
|
|
||||||
|
|
||||||
1. **Single "Trezorerie" Button**: Replace [Trezorerie Casa] and [Trezorerie Banca] with a single [Trezorerie] button in main menu
|
|
||||||
2. **Combined Display Format**: Show unified message with:
|
|
||||||
- Grand Total (Casa + Banca)
|
|
||||||
- Casa section: total + all accounts
|
|
||||||
- Banca section: total + all accounts
|
|
||||||
3. **Menu Layout Compaction**: Reorganize main menu to fill the freed space:
|
|
||||||
- Row 2: [Sold Companie] [Trezorerie]
|
|
||||||
- Row 3: [Sold Clienti] [Sold Furnizori]
|
|
||||||
- Row 4: [Evolutie Incasari]
|
|
||||||
4. **Backward Compatibility**: Keep `/trezorerie`, `/trezorerie_casa`, `/trezorerie_banca` commands working
|
|
||||||
5. **Performance Footer**: Include cache hit/miss metadata in response (consistent with existing pattern)
|
|
||||||
|
|
||||||
### Secondary Requirements
|
|
||||||
|
|
||||||
1. **Export Support**: Add "Export" button (matching other financial views)
|
|
||||||
2. **Refresh Support**: Add "Refresh" button (matching other financial views)
|
|
||||||
3. **Error Handling**: Graceful fallback if treasury data unavailable
|
|
||||||
|
|
||||||
## Technical Requirements
|
|
||||||
|
|
||||||
### Files to Modify
|
|
||||||
|
|
||||||
| File | Changes |
|
|
||||||
|------|---------|
|
|
||||||
| `backend/modules/telegram/bot/menus.py` | Update `create_main_menu()` (lines 234-247): Replace 2-button rows with unified layout |
|
|
||||||
| `backend/modules/telegram/bot/formatters.py` | Add `format_treasury_combined_response()` function after line 187 |
|
|
||||||
| `backend/modules/telegram/bot/handlers.py` | Update `button_callback()` (lines 1486-1546): Add `menu:trezorerie` case; Keep existing casa/banca cases for legacy commands |
|
|
||||||
| `backend/modules/telegram/bot/handlers.py` | Update `/trezorerie` command handler (if exists) to use new unified formatter, OR create new `trezorerie_unified_command()` |
|
|
||||||
|
|
||||||
### New Files to Create
|
|
||||||
|
|
||||||
None - all changes are modifications to existing files.
|
|
||||||
|
|
||||||
### Dependencies
|
|
||||||
|
|
||||||
- Existing: `get_treasury_breakdown_split()` from `helpers.py` (lines 275-354)
|
|
||||||
- Existing: `format_response_with_company()` from `menus.py` (lines 111-144)
|
|
||||||
- Existing: `create_action_buttons()` from `menus.py` (lines 278-335)
|
|
||||||
- Existing: `add_performance_footer()` from `formatters.py`
|
|
||||||
|
|
||||||
### Database Changes
|
|
||||||
|
|
||||||
None - uses existing `/api/reports/dashboard/treasury-breakdown` endpoint.
|
|
||||||
|
|
||||||
### API Changes
|
|
||||||
|
|
||||||
None - reuses existing backend API endpoints.
|
|
||||||
|
|
||||||
## Design Decisions
|
|
||||||
|
|
||||||
### Approach
|
|
||||||
|
|
||||||
**Unified Formatter Pattern**: Create a new `format_treasury_combined_response()` function that:
|
|
||||||
1. Takes the full `treasury_data` dict (containing both `casa` and `banca` keys)
|
|
||||||
2. Calculates grand total by summing casa + banca totals
|
|
||||||
3. Formats a single message with three sections: Grand Total, Casa breakdown, Banca breakdown
|
|
||||||
4. Reuses existing formatting patterns (Markdown bold, account lists, RON amounts)
|
|
||||||
|
|
||||||
**Menu Reorganization**: Compact the main menu layout to:
|
|
||||||
- Row 2: [Sold Companie] [Trezorerie] (unified button replaces Trezorerie Casa)
|
|
||||||
- Row 3: [Sold Clienti] [Sold Furnizori] (moves up from previous rows)
|
|
||||||
- Row 4: [Evolutie Incasari] (full width, moves up)
|
|
||||||
|
|
||||||
This creates a balanced 2-2-1 button layout that's more compact than the previous 2-2-2 layout.
|
|
||||||
|
|
||||||
**Callback Naming**: Use `menu:trezorerie` for the new unified button to maintain consistency with existing callback patterns (`menu:sold`, `menu:clienti`, etc.).
|
|
||||||
|
|
||||||
### Alternatives Considered
|
|
||||||
|
|
||||||
1. **Keep Both Buttons + Add Third**: Rejected because it increases menu clutter instead of reducing it
|
|
||||||
2. **Tabbed Interface**: Rejected because Telegram inline keyboards don't support tabs; would require complex state management
|
|
||||||
3. **Remove Legacy Commands**: Rejected to maintain backward compatibility for power users who have muscle memory for old commands
|
|
||||||
|
|
||||||
## Acceptance Criteria
|
|
||||||
|
|
||||||
### Menu Changes
|
|
||||||
- [ ] Main menu has single [Trezorerie] button instead of [Trezorerie Casa] and [Trezorerie Banca]
|
|
||||||
- [ ] Menu layout shows Row 2: [Sold Companie] [Trezorerie]
|
|
||||||
- [ ] Menu layout shows Row 3: [Sold Clienti] [Sold Furnizori]
|
|
||||||
- [ ] Menu layout shows Row 4: [Evolutie Incasari] (full width)
|
|
||||||
|
|
||||||
### Message Format
|
|
||||||
- [ ] Unified message shows "Sold Total Trezorerie: X,XXX RON" at top
|
|
||||||
- [ ] Unified message shows "Casa" section with total and account list
|
|
||||||
- [ ] Unified message shows "Banca" section with total and account list
|
|
||||||
- [ ] Grand total equals sum of Casa total + Banca total
|
|
||||||
- [ ] All amounts rounded to whole RON (0 decimals) with thousands separator
|
|
||||||
- [ ] Company name displayed at top of message
|
|
||||||
- [ ] Performance footer shows cache hit/miss metadata
|
|
||||||
|
|
||||||
### Functionality
|
|
||||||
- [ ] Tapping [Trezorerie] button displays unified treasury message
|
|
||||||
- [ ] Message includes [Refresh] and [Menu Principal] action buttons
|
|
||||||
- [ ] Legacy `/trezorerie_casa` command still works (shows Casa only)
|
|
||||||
- [ ] Legacy `/trezorerie_banca` command still works (Banca only)
|
|
||||||
- [ ] `/trezorerie` command shows unified view (if exists, otherwise create it)
|
|
||||||
- [ ] Callback `menu:trezorerie` triggers unified view
|
|
||||||
- [ ] Error handling works if treasury data unavailable
|
|
||||||
|
|
||||||
### Code Quality
|
|
||||||
- [ ] No code duplication between formatters
|
|
||||||
- [ ] Consistent Markdown formatting with existing patterns
|
|
||||||
- [ ] Proper error logging in handlers
|
|
||||||
- [ ] Comments explain new unified formatter logic
|
|
||||||
|
|
||||||
## Out of Scope
|
|
||||||
|
|
||||||
- **Export Functionality**: While we add the Export button, implementing actual Excel/PDF export is out of scope (deferred to future feature)
|
|
||||||
- **Historical Trends**: No graph or historical data - current snapshot only
|
|
||||||
- **Currency Conversion**: RON only, no multi-currency support
|
|
||||||
- **Account Filtering**: Show all accounts, no user-selectable filters
|
|
||||||
- **Menu Help Text**: No changes to `/help` command text (can be updated separately)
|
|
||||||
|
|
||||||
## Message Format Examples
|
|
||||||
|
|
||||||
### Unified Treasury Message (New)
|
|
||||||
|
|
||||||
```
|
|
||||||
Five Holding SRL
|
|
||||||
|
|
||||||
Sold Total Trezorerie: 20,500 RON
|
|
||||||
|
|
||||||
Casa
|
|
||||||
Sold Total Cash: 5,000 RON
|
|
||||||
|
|
||||||
Conturi de Casa:
|
|
||||||
- Casa Lei: 3,000 RON
|
|
||||||
- Casa Valuta: 2,000 RON
|
|
||||||
|
|
||||||
Banca
|
|
||||||
Sold Total Banca: 15,500 RON
|
|
||||||
|
|
||||||
Conturi Bancare:
|
|
||||||
- BCR RON: 10,000 RON
|
|
||||||
- BRD EUR: 5,500 RON
|
|
||||||
|
|
||||||
[Cache HIT | L1 | 23ms]
|
|
||||||
```
|
|
||||||
|
|
||||||
### Casa Only Message (Legacy `/trezorerie_casa`)
|
|
||||||
|
|
||||||
```
|
|
||||||
Five Holding SRL
|
|
||||||
|
|
||||||
Sold Total Cash: 5,000 RON
|
|
||||||
|
|
||||||
Conturi de Casa:
|
|
||||||
- Casa Lei: 3,000 RON
|
|
||||||
- Casa Valuta: 2,000 RON
|
|
||||||
|
|
||||||
[Cache HIT | L1 | 23ms]
|
|
||||||
```
|
|
||||||
|
|
||||||
### Banca Only Message (Legacy `/trezorerie_banca`)
|
|
||||||
|
|
||||||
```
|
|
||||||
Five Holding SRL
|
|
||||||
|
|
||||||
Sold Total Banca: 15,500 RON
|
|
||||||
|
|
||||||
Conturi Bancare:
|
|
||||||
- BCR RON: 10,000 RON
|
|
||||||
- BRD EUR: 5,500 RON
|
|
||||||
|
|
||||||
[Cache HIT | L1 | 23ms]
|
|
||||||
```
|
|
||||||
|
|
||||||
## Implementation Details
|
|
||||||
|
|
||||||
### 1. New Unified Formatter
|
|
||||||
|
|
||||||
Add to `backend/modules/telegram/bot/formatters.py` after line 187:
|
|
||||||
|
|
||||||
```python
|
|
||||||
def format_treasury_combined_response(data: Dict[str, Any], company_name: str = None) -> str:
|
|
||||||
"""
|
|
||||||
Format combined treasury data (Casa + Banca) for Telegram.
|
|
||||||
|
|
||||||
Args:
|
|
||||||
data: Dict with 'casa' and 'banca' keys from get_treasury_breakdown_split()
|
|
||||||
company_name: Company name (kept for compatibility, not used)
|
|
||||||
|
|
||||||
Returns:
|
|
||||||
Formatted Markdown string with grand total and both sections
|
|
||||||
|
|
||||||
Example:
|
|
||||||
data = {'casa': {...}, 'banca': {...}}
|
|
||||||
text = format_treasury_combined_response(data)
|
|
||||||
"""
|
|
||||||
text = ""
|
|
||||||
|
|
||||||
# Extract totals
|
|
||||||
casa_total = round(data.get('casa', {}).get('total', 0))
|
|
||||||
banca_total = round(data.get('banca', {}).get('total', 0))
|
|
||||||
grand_total = casa_total + banca_total
|
|
||||||
|
|
||||||
# Grand total
|
|
||||||
text += f"**Sold Total Trezorerie:** {grand_total:,} RON\n\n"
|
|
||||||
|
|
||||||
# Casa section
|
|
||||||
text += "**Casa**\n"
|
|
||||||
text += f"Sold Total Cash: {casa_total:,} RON\n\n"
|
|
||||||
|
|
||||||
casa_accounts = data.get('casa', {}).get('accounts', [])
|
|
||||||
if casa_accounts:
|
|
||||||
text += "Conturi de Casa:\n"
|
|
||||||
for acc in casa_accounts:
|
|
||||||
name = acc.get('name', 'N/A')
|
|
||||||
balance = round(acc.get('balance', 0))
|
|
||||||
text += f" - {name}: {balance:,} RON\n"
|
|
||||||
else:
|
|
||||||
text += "Nu exista conturi de casa.\n"
|
|
||||||
|
|
||||||
text += "\n"
|
|
||||||
|
|
||||||
# Banca section
|
|
||||||
text += "**Banca**\n"
|
|
||||||
text += f"Sold Total Banca: {banca_total:,} RON\n\n"
|
|
||||||
|
|
||||||
banca_accounts = data.get('banca', {}).get('accounts', [])
|
|
||||||
if banca_accounts:
|
|
||||||
text += "Conturi Bancare:\n"
|
|
||||||
for acc in banca_accounts:
|
|
||||||
name = acc.get('name', 'N/A')
|
|
||||||
balance = round(acc.get('balance', 0))
|
|
||||||
text += f" - {name}: {balance:,} RON\n"
|
|
||||||
else:
|
|
||||||
text += "Nu exista conturi bancare.\n"
|
|
||||||
|
|
||||||
return text
|
|
||||||
```
|
|
||||||
|
|
||||||
### 2. Update Main Menu Layout
|
|
||||||
|
|
||||||
Update `backend/modules/telegram/bot/menus.py` lines 234-247:
|
|
||||||
|
|
||||||
```python
|
|
||||||
# Rows 2-4: Financial options (compacted layout)
|
|
||||||
keyboard.extend([
|
|
||||||
[
|
|
||||||
InlineKeyboardButton("Sold Companie", callback_data="menu:sold"),
|
|
||||||
InlineKeyboardButton("Trezorerie", callback_data="menu:trezorerie")
|
|
||||||
],
|
|
||||||
[
|
|
||||||
InlineKeyboardButton("Sold Clienti", callback_data="menu:clienti"),
|
|
||||||
InlineKeyboardButton("Sold Furnizori", callback_data="menu:furnizori")
|
|
||||||
],
|
|
||||||
[
|
|
||||||
InlineKeyboardButton("Evolutie Incasari", callback_data="menu:evolutie")
|
|
||||||
]
|
|
||||||
])
|
|
||||||
```
|
|
||||||
|
|
||||||
### 3. Update Button Callback Handler
|
|
||||||
|
|
||||||
Update `backend/modules/telegram/bot/handlers.py` in `button_callback()` function, add new case after line 1485:
|
|
||||||
|
|
||||||
```python
|
|
||||||
elif action == "trezorerie":
|
|
||||||
# Unified trezorerie (Casa + Banca combined)
|
|
||||||
from backend.modules.telegram.bot.helpers import get_treasury_breakdown_split
|
|
||||||
treasury_data = await get_treasury_breakdown_split(company['id'], jwt_token)
|
|
||||||
|
|
||||||
from backend.modules.telegram.bot.formatters import format_treasury_combined_response, add_performance_footer
|
|
||||||
from backend.modules.telegram.bot.menus import create_action_buttons, format_response_with_company
|
|
||||||
|
|
||||||
content = format_treasury_combined_response(treasury_data)
|
|
||||||
response = format_response_with_company(content, company['name'])
|
|
||||||
|
|
||||||
# Add performance footer if cache metadata is available
|
|
||||||
if 'cache_hit' in treasury_data and 'response_time_ms' in treasury_data:
|
|
||||||
cache_hit = treasury_data['cache_hit']
|
|
||||||
response_time_ms = treasury_data['response_time_ms']
|
|
||||||
cache_source = treasury_data.get('cache_source', None)
|
|
||||||
response = add_performance_footer(response, cache_hit, response_time_ms, cache_source)
|
|
||||||
|
|
||||||
keyboard = create_action_buttons("trezorerie", show_export=False, show_refresh=False)
|
|
||||||
|
|
||||||
try:
|
|
||||||
await query.edit_message_text(
|
|
||||||
response,
|
|
||||||
reply_markup=keyboard,
|
|
||||||
parse_mode=ParseMode.MARKDOWN
|
|
||||||
)
|
|
||||||
except Exception as e:
|
|
||||||
# Ignore "Message is not modified" error
|
|
||||||
if "Message is not modified" not in str(e):
|
|
||||||
raise
|
|
||||||
|
|
||||||
elif action == "casa":
|
|
||||||
# Keep existing casa handler for legacy /trezorerie_casa command
|
|
||||||
# ... existing code ...
|
|
||||||
```
|
|
||||||
|
|
||||||
### 4. Legacy Command Handlers
|
|
||||||
|
|
||||||
Keep existing handlers in `backend/modules/telegram/bot/handlers.py`:
|
|
||||||
- `trezorerie_casa_command()` (lines 884-955) - NO CHANGES
|
|
||||||
- `trezorerie_banca_command()` (lines 957-1028) - NO CHANGES
|
|
||||||
|
|
||||||
Update `/trezorerie` command (or create if missing) to use unified formatter:
|
|
||||||
|
|
||||||
```python
|
|
||||||
async def trezorerie_command(update: Update, context: ContextTypes.DEFAULT_TYPE):
|
|
||||||
"""
|
|
||||||
Handle /trezorerie command - shows unified treasury data (Casa + Banca).
|
|
||||||
|
|
||||||
Displays complete treasury overview with grand total and account breakdowns.
|
|
||||||
|
|
||||||
Args:
|
|
||||||
update: Telegram update object
|
|
||||||
context: Telegram context
|
|
||||||
"""
|
|
||||||
try:
|
|
||||||
telegram_user_id = update.effective_user.id
|
|
||||||
logger.info(f"/trezorerie command from user {telegram_user_id}")
|
|
||||||
|
|
||||||
# Check linked
|
|
||||||
is_linked = await check_user_linked(telegram_user_id)
|
|
||||||
if not is_linked:
|
|
||||||
await update.message.reply_text(
|
|
||||||
"**Cont neconectat**\n\nFoloseste /start",
|
|
||||||
parse_mode=ParseMode.MARKDOWN
|
|
||||||
)
|
|
||||||
return
|
|
||||||
|
|
||||||
# Get active company
|
|
||||||
session_manager = get_session_manager()
|
|
||||||
from backend.modules.telegram.bot.helpers import get_active_company_or_prompt
|
|
||||||
company = await get_active_company_or_prompt(update, session_manager, telegram_user_id)
|
|
||||||
|
|
||||||
if not company:
|
|
||||||
return # Prompt already sent
|
|
||||||
|
|
||||||
# Get auth data
|
|
||||||
auth_data = await get_user_auth_data(telegram_user_id)
|
|
||||||
jwt_token = auth_data['jwt_token']
|
|
||||||
|
|
||||||
# Get treasury breakdown split
|
|
||||||
from backend.modules.telegram.bot.helpers import get_treasury_breakdown_split
|
|
||||||
treasury_data = await get_treasury_breakdown_split(
|
|
||||||
company_id=company['id'],
|
|
||||||
jwt_token=jwt_token
|
|
||||||
)
|
|
||||||
|
|
||||||
if not treasury_data:
|
|
||||||
await update.message.reply_text("Eroare la incarcarea trezoreriei.")
|
|
||||||
return
|
|
||||||
|
|
||||||
# Format unified response
|
|
||||||
from backend.modules.telegram.bot.formatters import format_treasury_combined_response, add_performance_footer
|
|
||||||
from backend.modules.telegram.bot.menus import create_action_buttons, format_response_with_company
|
|
||||||
|
|
||||||
content = format_treasury_combined_response(treasury_data)
|
|
||||||
response = format_response_with_company(content, company['name'])
|
|
||||||
|
|
||||||
# Add performance footer if cache metadata is available
|
|
||||||
if 'cache_hit' in treasury_data and 'response_time_ms' in treasury_data:
|
|
||||||
cache_hit = treasury_data['cache_hit']
|
|
||||||
response_time_ms = treasury_data['response_time_ms']
|
|
||||||
cache_source = treasury_data.get('cache_source', None)
|
|
||||||
response = add_performance_footer(response, cache_hit, response_time_ms, cache_source)
|
|
||||||
|
|
||||||
keyboard = create_action_buttons("trezorerie", show_export=True)
|
|
||||||
|
|
||||||
await update.message.reply_text(
|
|
||||||
response,
|
|
||||||
reply_markup=keyboard,
|
|
||||||
parse_mode=ParseMode.MARKDOWN
|
|
||||||
)
|
|
||||||
|
|
||||||
except Exception as e:
|
|
||||||
logger.error(f"Error in trezorerie_command: {e}", exc_info=True)
|
|
||||||
await update.message.reply_text("Eroare la incarcarea trezoreriei.")
|
|
||||||
```
|
|
||||||
|
|
||||||
### 5. Command Registration
|
|
||||||
|
|
||||||
Update `backend/modules/telegram/bot_main.py` - ensure `trezorerie_command` is registered (line 126):
|
|
||||||
|
|
||||||
```python
|
|
||||||
application.add_handler(CommandHandler("trezorerie", trezorerie_command))
|
|
||||||
```
|
|
||||||
|
|
||||||
No changes needed for `trezorerie_casa_command` and `trezorerie_banca_command` (already registered at lines 127-128).
|
|
||||||
|
|
||||||
## Risks and Mitigations
|
|
||||||
|
|
||||||
| Risk | Likelihood | Impact | Mitigation |
|
|
||||||
|------|------------|--------|------------|
|
|
||||||
| Users confused by menu change | Medium | Low | Keep legacy commands working; users can still use old commands if preferred |
|
|
||||||
| Grand total calculation error | Low | Medium | Add unit tests verifying `casa_total + banca_total = grand_total`; Use `round()` consistently |
|
|
||||||
| Message too long for Telegram | Low | Medium | Telegram limit is 4096 chars; current format uses ~300-500 chars; Monitor in production |
|
|
||||||
| Cache metadata missing | Low | Low | Graceful fallback: only add footer if metadata exists (existing pattern) |
|
|
||||||
| Formatting inconsistencies | Low | Low | Reuse existing formatters (`format_response_with_company`, `add_performance_footer`) |
|
|
||||||
|
|
||||||
## Open Questions
|
|
||||||
|
|
||||||
1. **Should `/trezorerie` show unified view or redirect to menu?**
|
|
||||||
- **Decision**: Show unified view (recommended for consistency)
|
|
||||||
- Rationale: More useful for power users who type commands
|
|
||||||
|
|
||||||
2. **Should we add Export button functionality now or later?**
|
|
||||||
- **Decision**: Add button now, implement export functionality later
|
|
||||||
- Rationale: Maintains UI consistency with other views; export can be separate feature
|
|
||||||
|
|
||||||
3. **Should we update `/help` command text to reflect menu changes?**
|
|
||||||
- **Decision**: Out of scope for this feature
|
|
||||||
- Rationale: Can be updated in separate UX improvement task
|
|
||||||
|
|
||||||
## Testing Strategy
|
|
||||||
|
|
||||||
### Manual Testing Checklist
|
|
||||||
|
|
||||||
1. **Menu Navigation**:
|
|
||||||
- [ ] /menu shows new compact layout
|
|
||||||
- [ ] [Trezorerie] button exists in Row 2
|
|
||||||
- [ ] [Trezorerie Casa] and [Trezorerie Banca] buttons removed
|
|
||||||
- [ ] [Sold Clienti] and [Sold Furnizori] in Row 3
|
|
||||||
- [ ] [Evolutie Incasari] in Row 4
|
|
||||||
|
|
||||||
2. **Unified View**:
|
|
||||||
- [ ] Tapping [Trezorerie] shows combined message
|
|
||||||
- [ ] Grand total = Casa total + Banca total
|
|
||||||
- [ ] Casa section shows total + accounts
|
|
||||||
- [ ] Banca section shows total + accounts
|
|
||||||
- [ ] Company name at top
|
|
||||||
- [ ] Performance footer at bottom
|
|
||||||
|
|
||||||
3. **Legacy Commands**:
|
|
||||||
- [ ] `/trezorerie_casa` shows Casa only
|
|
||||||
- [ ] `/trezorerie_banca` shows Banca only
|
|
||||||
- [ ] `/trezorerie` shows unified view
|
|
||||||
|
|
||||||
4. **Action Buttons**:
|
|
||||||
- [ ] [Refresh] button exists (for command, not callback)
|
|
||||||
- [ ] [Menu Principal] button exists
|
|
||||||
- [ ] [Menu Principal] returns to main menu
|
|
||||||
|
|
||||||
5. **Edge Cases**:
|
|
||||||
- [ ] No casa accounts: Shows "Nu exista conturi de casa"
|
|
||||||
- [ ] No banca accounts: Shows "Nu exista conturi bancare"
|
|
||||||
- [ ] Zero balances: Shows "0 RON"
|
|
||||||
- [ ] Large amounts: Thousands separator works (e.g., "1,234,567 RON")
|
|
||||||
|
|
||||||
### Unit Testing
|
|
||||||
|
|
||||||
```python
|
|
||||||
# Test unified formatter
|
|
||||||
def test_format_treasury_combined_response():
|
|
||||||
data = {
|
|
||||||
'casa': {
|
|
||||||
'total': 5000.0,
|
|
||||||
'accounts': [
|
|
||||||
{'name': 'Casa Lei', 'balance': 3000.0},
|
|
||||||
{'name': 'Casa Valuta', 'balance': 2000.0}
|
|
||||||
]
|
|
||||||
},
|
|
||||||
'banca': {
|
|
||||||
'total': 15500.0,
|
|
||||||
'accounts': [
|
|
||||||
{'name': 'BCR RON', 'balance': 10000.0},
|
|
||||||
{'name': 'BRD EUR', 'balance': 5500.0}
|
|
||||||
]
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
result = format_treasury_combined_response(data)
|
|
||||||
|
|
||||||
# Assert grand total
|
|
||||||
assert "20,500 RON" in result
|
|
||||||
|
|
||||||
# Assert casa section
|
|
||||||
assert "Casa Lei: 3,000 RON" in result
|
|
||||||
assert "Casa Valuta: 2,000 RON" in result
|
|
||||||
|
|
||||||
# Assert banca section
|
|
||||||
assert "BCR RON: 10,000 RON" in result
|
|
||||||
assert "BRD EUR: 5,500 RON" in result
|
|
||||||
```
|
|
||||||
|
|
||||||
## Estimated Complexity
|
|
||||||
|
|
||||||
**Medium** - This is a straightforward refactoring task with clear requirements and existing patterns to follow.
|
|
||||||
|
|
||||||
**Justification**:
|
|
||||||
- **Low Risk**: No database changes, no new API endpoints, uses existing data
|
|
||||||
- **Well-Defined**: Clear specification with examples and acceptance criteria
|
|
||||||
- **Existing Patterns**: Follows established formatter and handler patterns
|
|
||||||
- **Backward Compatible**: Legacy commands remain functional
|
|
||||||
- **Estimated Effort**: 2-3 hours (1h coding, 1h testing, 0.5h code review)
|
|
||||||
|
|
||||||
**Complexity Breakdown**:
|
|
||||||
- New formatter function: 30 min (straightforward string formatting)
|
|
||||||
- Menu layout update: 10 min (simple button rearrangement)
|
|
||||||
- Callback handler: 20 min (copy-paste existing pattern)
|
|
||||||
- Legacy command update: 20 min (if `/trezorerie` needs changes)
|
|
||||||
- Testing: 60 min (manual testing + edge cases)
|
|
||||||
- Code review fixes: 30 min (buffer for feedback)
|
|
||||||
|
|
||||||
**Total Estimated Time**: 2.5 hours
|
|
||||||
@@ -1,30 +0,0 @@
|
|||||||
{
|
|
||||||
"feature": "telegram-trezorerie",
|
|
||||||
"status": "IMPLEMENTATION_COMPLETE",
|
|
||||||
"created_at": "2025-12-30T18:30:00Z",
|
|
||||||
"updated": "2025-12-30T18:51:00Z",
|
|
||||||
"totalTasks": 4,
|
|
||||||
"currentTask": 4,
|
|
||||||
"tasksCompleted": 4,
|
|
||||||
"estimated_complexity": "medium",
|
|
||||||
"estimated_hours": 2.5,
|
|
||||||
"files_affected": 3,
|
|
||||||
"requires_database_changes": false,
|
|
||||||
"requires_api_changes": false,
|
|
||||||
"backward_compatible": true,
|
|
||||||
"history": [
|
|
||||||
{"status": "SPEC_DRAFT", "at": "2025-12-30T18:30:00Z"},
|
|
||||||
{"status": "SPEC_COMPLETE", "at": "2025-12-30T18:35:00Z"},
|
|
||||||
{"status": "PLANNING", "at": "2025-12-30T18:45:00Z"},
|
|
||||||
{"status": "PLANNING_COMPLETE", "at": "2025-12-30T18:46:00Z"},
|
|
||||||
{"status": "IMPLEMENTING", "at": "2025-12-30T18:47:00Z", "task": 1, "started": true},
|
|
||||||
{"status": "IMPLEMENTING", "at": "2025-12-30T18:48:00Z", "task": 1, "title": "Add unified formatter", "completed": true},
|
|
||||||
{"status": "IMPLEMENTING", "at": "2025-12-30T18:48:00Z", "task": 2, "started": true},
|
|
||||||
{"status": "IMPLEMENTING", "at": "2025-12-30T18:49:00Z", "task": 2, "title": "Update main menu layout", "completed": true},
|
|
||||||
{"status": "IMPLEMENTING", "at": "2025-12-30T18:49:00Z", "task": 3, "started": true},
|
|
||||||
{"status": "IMPLEMENTING", "at": "2025-12-30T18:50:00Z", "task": 3, "title": "Add callback handler", "completed": true},
|
|
||||||
{"status": "IMPLEMENTING", "at": "2025-12-30T18:50:00Z", "task": 4, "started": true},
|
|
||||||
{"status": "IMPLEMENTING", "at": "2025-12-30T18:51:00Z", "task": 4, "title": "Manual testing", "completed": true},
|
|
||||||
{"status": "IMPLEMENTATION_COMPLETE", "at": "2025-12-30T18:51:00Z"}
|
|
||||||
]
|
|
||||||
}
|
|
||||||
29
.gitignore
vendored
29
.gitignore
vendored
@@ -433,15 +433,9 @@ run_tests.*
|
|||||||
scan_*.json
|
scan_*.json
|
||||||
sdist/
|
sdist/
|
||||||
sdist/
|
sdist/
|
||||||
# Secrets directories (contains credentials, keys, passwords)
|
|
||||||
secrets/
|
secrets/
|
||||||
|
|
||||||
# Allow documentation in secrets directories
|
# Allow documentation in secrets directories
|
||||||
!**/secrets/README.md
|
!**/secrets/README.md
|
||||||
|
|
||||||
# SSH tunnel configuration (next to .env files)
|
|
||||||
backend/ssh-tunnels.json
|
|
||||||
!backend/ssh-tunnels.json.example
|
|
||||||
security_*.json
|
security_*.json
|
||||||
share/python-wheels/
|
share/python-wheels/
|
||||||
sqlnet.ora
|
sqlnet.ora
|
||||||
@@ -531,26 +525,5 @@ backend/data/receipts/uploads/*
|
|||||||
backend/data/ocr_queue/
|
backend/data/ocr_queue/
|
||||||
!backend/data/*/.gitkeep
|
!backend/data/*/.gitkeep
|
||||||
|
|
||||||
# Auth trusted devices (conține date sensibile — nu commita!)
|
# Handoff document (session continuity, not for version control)
|
||||||
data/auth/trusted_devices.json
|
|
||||||
|
|
||||||
# PRD tasks (generated, not tracked)
|
|
||||||
tasks/
|
|
||||||
|
|
||||||
# ============================================================================
|
|
||||||
# 🤖 CLAUDE & RALPH AUTOMATION - DO NOT COMMIT
|
|
||||||
# ============================================================================
|
|
||||||
# Claude handover and context files (session-specific)
|
|
||||||
.claude/HANDOFF.md
|
.claude/HANDOFF.md
|
||||||
.claude/handover/
|
|
||||||
CONTEXT_HANDOVER_*.md
|
|
||||||
|
|
||||||
# Ralph automation - ignore entire directory
|
|
||||||
scripts/ralph/
|
|
||||||
|
|
||||||
# ============================================================================
|
|
||||||
# 💾 SQLITE RUNTIME FILES - DO NOT COMMIT
|
|
||||||
# ============================================================================
|
|
||||||
# SQLite WAL (Write-Ahead Log) and SHM (Shared Memory) files
|
|
||||||
*.db-shm
|
|
||||||
*.db-wal
|
|
||||||
|
|||||||
Reference in New Issue
Block a user