add chunking

docs/02_components/chunking_system_assessment.md

@@ -0,0 +1,440 @@
+# Chunking System Assessment: F12 Route Chunk Manager and Integration
+
+## Executive Summary
+
+The chunking system implements the Atlas multi-map architecture from the solution document, with **F12 Route Chunk Manager** serving as the high-level coordinator. The system demonstrates **strong coherence** with clear separation of concerns between F12 (lifecycle), F10 (factor graph), and F11 (matching coordination). However, there are some **integration inconsistencies** and **missing lifecycle transitions** that should be addressed.
+
+**Overall Assessment: 8/10** - Well-designed with minor gaps
+
+---
+
+## 1. Architecture Coherence
+
+### 1.1 Separation of Concerns ✅
+
+The chunking system properly separates responsibilities:
+
+| Component | Responsibility | Level |
+|-----------|---------------|-------|
+| **F12 Route Chunk Manager** | Chunk lifecycle, state tracking, matching coordination | High-level (flight context) |
+| **F10 Factor Graph Optimizer** | Chunk subgraphs, factor management, optimization | Low-level (factor graph) |
+| **F11 Failure Recovery Coordinator** | Chunk creation triggers, matching orchestration | Coordination layer |
+| **F02 Flight Processor** | Chunk-aware frame processing | Application layer |
+
+**Assessment:** ✅ **Excellent separation** - Clear boundaries, no overlap
+
+### 1.2 Chunk Lifecycle Completeness
+
+**Lifecycle States:**
+- `unanchored` → `matching` → `anchored` → `merged`
+
+**Lifecycle Methods:**
+
+| State Transition | F12 Method | F10 Method | Status |
+|------------------|------------|------------|--------|
+| **Creation** | `create_chunk()` | `create_new_chunk()` | ✅ Complete |
+| **Building** | `add_frame_to_chunk()` | `add_relative_factor_to_chunk()` | ✅ Complete |
+| **Matching** | `is_chunk_ready_for_matching()` | N/A | ✅ Complete |
+| **Anchoring** | `mark_chunk_anchored()` | `add_chunk_anchor()` | ✅ Complete |
+| **Merging** | `deactivate_chunk()` | `merge_chunks()` | ⚠️ **Gap** |
+| **Deactivation** | `deactivate_chunk()` | N/A | ✅ Complete |
+
+**Status:** ✅ **Fixed** - Added `F12.merge_chunks()` method for chunk merging coordination
+
+---
+
+## 2. Integration Analysis
+
+### 2.1 F12 ↔ F10 Integration
+
+**F12 calls F10:**
+- ✅ `create_chunk()` → `F10.create_new_chunk()` - Correct
+- ✅ `add_frame_to_chunk()` → `F10.add_relative_factor_to_chunk()` - Correct
+- ✅ `mark_chunk_anchored()` → `F10.add_chunk_anchor()` - Correct
+- ✅ `get_chunk_bounds()` → `F10.get_chunk_trajectory()` - Correct
+
+**F10 exposes chunk operations:**
+- ✅ `create_new_chunk()` - Low-level factor graph operation
+- ✅ `add_relative_factor_to_chunk()` - Factor management
+- ✅ `add_chunk_anchor()` - Anchor management
+- ✅ `merge_chunks()` - Sim(3) transformation
+- ✅ `optimize_chunk()` - Chunk-level optimization
+- ✅ `get_chunk_trajectory()` - Trajectory retrieval
+
+**Assessment:** ✅ **Well-integrated** - F12 properly wraps F10 operations
+
+**Status:** ✅ **Fixed** - F10's method renamed to `get_chunk_for_frame(frame_id)` for clarity, F12's `get_active_chunk(flight_id)` remains for high-level queries.
+
+---
+
+### 2.2 F12 ↔ F11 Integration
+
+**F11 calls F12:**
+- ✅ `create_chunk_on_tracking_loss()` → `F12.create_chunk()` - Correct (proactive)
+- ✅ `try_chunk_semantic_matching()` → Uses F12 methods indirectly - Correct
+- ✅ `try_chunk_litesam_matching()` → Uses F12 methods indirectly - Correct
+- ✅ `merge_chunk_to_trajectory()` → Calls `F10.merge_chunks()` directly - ⚠️ **Bypasses F12**
+
+**F12 provides for F11:**
+- ✅ `get_chunks_for_matching()` - Returns unanchored chunks
+- ✅ `get_chunk_images()` - Image retrieval
+- ✅ `get_chunk_composite_descriptor()` - Descriptor computation
+- ✅ `get_chunk_bounds()` - Bounds for tile search
+
+**Assessment:** ⚠️ **Minor inconsistency** - F11 bypasses F12 for merging
+
+**Status:** ✅ **Fixed** - F11 now calls `F12.merge_chunks()` which coordinates with F10 and updates chunk states.
+
+---
+
+### 2.3 F12 ↔ F02 Integration
+
+**F02 calls F12:**
+- ✅ `get_active_chunk(flight_id)` - Before processing frame
+- ✅ `create_new_chunk(flight_id, frame_id)` - On tracking loss
+- ✅ `add_frame_to_chunk()` - During frame processing
+
+**F02 chunk-aware processing:**
+- ✅ Gets active chunk before processing frame
+- ✅ Creates new chunk on tracking loss
+- ✅ Adds frames to chunk with VO results
+
+**Assessment:** ✅ **Well-integrated** - F02 properly uses F12 for chunk management
+
+---
+
+### 2.4 F12 ↔ F08/F09 Integration
+
+**F08 Global Place Recognition:**
+- ✅ `get_chunk_images()` - Retrieves chunk images
+- ✅ `get_chunk_composite_descriptor()` - Gets aggregate descriptor
+- ⚠️ **Issue:** F12's `get_chunk_composite_descriptor()` calls `F08.compute_location_descriptor()` for each image, but F08 also has `compute_chunk_descriptor()` method. This creates duplication.
+
+**F09 Metric Refinement:**
+- ✅ `get_chunk_images()` - Retrieves chunk images
+- ✅ `align_chunk_to_satellite()` - Chunk-to-satellite matching
+
+**Assessment:** ⚠️ **Descriptor duplication** - F12 and F08 both compute chunk descriptors
+
+---
+
+## 3. Chunk Lifecycle Flow Analysis
+
+### 3.1 Normal Chunk Lifecycle
+
+```
+1. Tracking Lost (F02/F11 detects)
+   ↓
+2. create_chunk() (F12) → create_new_chunk() (F10)
+   ↓
+3. add_frame_to_chunk() × N (F12) → add_relative_factor_to_chunk() (F10)
+   ↓
+4. is_chunk_ready_for_matching() (F12) → True (>=5 frames)
+   ↓
+5. get_chunks_for_matching() (F12) → Returns chunk
+   ↓
+6. try_chunk_semantic_matching() (F11) → Uses F12.get_chunk_composite_descriptor()
+   ↓
+7. try_chunk_litesam_matching() (F11) → Uses F12.get_chunk_images()
+   ↓
+8. mark_chunk_anchored() (F12) → add_chunk_anchor() (F10)
+   ↓
+9. merge_chunks() (F10) → Called directly by F11 (bypasses F12)
+   ↓
+10. deactivate_chunk() (F12) → Chunk marked as merged
+```
+
+**Status:** ✅ **Fixed** - Step 9 now goes through F12.merge_chunks() maintaining abstraction
+
+---
+
+### 3.2 Proactive Chunk Creation
+
+**Solution Requirement:** Chunks created proactively on tracking loss, not reactively after matching failures.
+
+**Implementation:**
+- ✅ F11's `create_chunk_on_tracking_loss()` creates chunk immediately
+- ✅ F02's `handle_tracking_loss()` creates chunk proactively
+- ✅ Processing continues in new chunk while matching happens asynchronously
+
+**Assessment:** ✅ **Correctly implements proactive creation**
+
+---
+
+### 3.3 Chunk Matching Strategy
+
+**Solution Requirement:** Chunk semantic matching (aggregate DINOv2) → Chunk LiteSAM matching (rotation sweeps) → Chunk merging (Sim3)
+
+**Implementation:**
+- ✅ F12 provides `get_chunk_composite_descriptor()` for semantic matching
+- ✅ F11 coordinates semantic matching via F08
+- ✅ F11 coordinates LiteSAM matching via F09 with rotation sweeps
+- ✅ F10 provides `merge_chunks()` with Sim(3) transform
+
+**Assessment:** ✅ **Correctly implements matching strategy**
+
+---
+
+## 4. State Management Coherence
+
+### 4.1 ChunkHandle State Fields
+
+**F12 ChunkHandle:**
+```python
+chunk_id: str
+flight_id: str
+start_frame_id: int
+end_frame_id: Optional[int]
+frames: List[int]
+is_active: bool
+has_anchor: bool
+anchor_frame_id: Optional[int]
+anchor_gps: Optional[GPSPoint]
+matching_status: str  # "unanchored", "matching", "anchored", "merged"
+```
+
+**Assessment:** ✅ **Complete state representation** - All necessary fields present
+
+**Issue:** `matching_status` and `has_anchor` are redundant (if `matching_status == "anchored"`, then `has_anchor == True`). Consider consolidating.
+
+---
+
+### 4.2 State Transitions
+
+**Valid Transitions:**
+- `unanchored` → `matching` (when matching starts)
+- `matching` → `anchored` (when anchor found)
+- `anchored` → `merged` (when merged to global trajectory)
+- `unanchored` → `anchored` (direct anchor, e.g., user input)
+
+**Assessment:** ✅ **State transitions are well-defined**
+
+**Status:** ✅ **Fixed** - Added `F12.mark_chunk_matching(chunk_id)` method for explicit state transitions.
+
+---
+
+## 5. Missing Functionality
+
+### 5.1 Chunk Merging Coordination
+
+**Gap:** F12 doesn't have a method to coordinate chunk merging.
+
+**Current:** F11 calls `F10.merge_chunks()` directly, bypassing F12.
+
+**Recommendation:** Add `F12.merge_chunks(chunk_id_1, chunk_id_2, transform)` that:
+1. Validates chunks can be merged
+2. Calls `F10.merge_chunks()`
+3. Updates chunk states (deactivates chunk_id_1, updates chunk_id_2)
+4. Updates `matching_status` to "merged"
+
+---
+
+### 5.2 Chunk State Persistence
+
+**Gap:** No explicit persistence of chunk state.
+
+**Current:** Chunk state is in-memory only (via F12 and F10).
+
+**Recommendation:** F12 should persist chunk state to F03 Flight Database for:
+- Recovery after system restart
+- Chunk state queries
+- Debugging and analysis
+
+---
+
+### 5.3 Chunk Matching Status Updates
+
+**Gap:** No explicit method to update `matching_status` to "matching".
+
+**Current:** Status transitions happen implicitly in F11.
+
+**Recommendation:** Add `F12.mark_chunk_matching(chunk_id)` to explicitly track matching state.
+
+---
+
+## 6. Inconsistencies
+
+### 6.1 Descriptor Computation Duplication
+
+**Issue:** Both F08 and F12 compute chunk descriptors.
+
+- **F08:** `compute_chunk_descriptor(chunk_images)` - Computes aggregate DINOv2 descriptor
+- **F12:** `get_chunk_composite_descriptor(chunk_id)` - Also computes aggregate descriptor
+
+**Current Implementation (F12):**
+```python
+1. Get chunk images via get_chunk_images()
+2. For each image:
+   - Call F08.compute_location_descriptor(image) → descriptor
+3. Aggregate descriptors (mean, max, or VLAD)
+```
+
+**Status:** ✅ **Fixed** - F12 now calls `F08.compute_chunk_descriptor()` directly, eliminating duplication.
+
+---
+
+### 6.2 Active Chunk Query Inconsistency
+
+**Issue:** F10 and F12 have different signatures for `get_active_chunk()`.
+
+- **F10:** `get_active_chunk(frame_id: int)` - Query by frame ID
+- **F12:** `get_active_chunk(flight_id: str)` - Query by flight ID
+
+**Status:** ✅ **Fixed** - F10's method renamed to `get_chunk_for_frame(frame_id)` for clarity, clearly distinguishing from F12's `get_active_chunk(flight_id)`.
+
+---
+
+### 6.3 Chunk Merging Bypass
+
+**Issue:** F11 bypasses F12 when merging chunks.
+
+**Current:** `F11.merge_chunk_to_trajectory()` → `F10.merge_chunks()` directly
+
+**Status:** ✅ **Fixed** - F11 now calls `F12.merge_chunks()` which properly coordinates merging and updates chunk states.
+
+---
+
+## 7. Strengths
+
+### 7.1 Clear Abstraction Layers ✅
+
+- **F12** provides high-level chunk lifecycle management
+- **F10** provides low-level factor graph operations
+- Clear separation of concerns
+
+### 7.2 Proactive Chunk Creation ✅
+
+- Chunks created immediately on tracking loss
+- Processing continues while matching happens asynchronously
+- Matches solution requirement perfectly
+
+### 7.3 Complete Chunk State Tracking ✅
+
+- ChunkHandle captures all necessary state
+- State transitions are well-defined
+- Matching status tracks chunk progress
+
+### 7.4 Chunk Matching Integration ✅
+
+- F12 provides chunk representations (images, descriptors, bounds)
+- F11 coordinates matching strategies
+- F08/F09 perform actual matching
+
+### 7.5 Chunk Isolation ✅
+
+- Each chunk has independent subgraph in F10
+- Factors isolated to chunks
+- Local optimization before global merging
+
+---
+
+## 8. Weaknesses
+
+### 8.1 Missing Merging Coordination ✅ **FIXED**
+
+- ✅ F12.merge_chunks() method added
+- ✅ F11 now calls F12, maintaining abstraction
+- ✅ State updates handled by F12
+
+### 8.2 Descriptor Computation Duplication ✅ **FIXED**
+
+- ✅ F12 now delegates to F08.compute_chunk_descriptor() directly
+- ✅ Single source of truth for chunk descriptor computation
+
+### 8.3 No State Persistence ⚠️
+
+- Chunk state is in-memory only
+- No recovery after restart
+- No debugging/analysis capabilities
+- **Note:** This is a Priority 2 enhancement, not critical
+
+### 8.4 Implicit State Transitions ✅ **FIXED**
+
+- ✅ F12.mark_chunk_matching() method added
+- ✅ Explicit state transitions for matching status
+- ✅ Easier to track chunk state changes
+
+---
+
+## 9. Recommendations
+
+### Priority 1: Critical Fixes ✅ **COMPLETED**
+
+1. ✅ **Add F12.merge_chunks() method** - **FIXED**
+   - F12.merge_chunks() method added
+   - Coordinates chunk merging and updates states
+   - F11 now calls F12, maintaining abstraction
+
+2. ✅ **Fix descriptor computation duplication** - **FIXED**
+   - F12.get_chunk_composite_descriptor() now calls F08.compute_chunk_descriptor() directly
+   - Removed duplicate aggregation logic from F12
+
+3. ✅ **Add explicit matching status updates** - **FIXED**
+   - F12.mark_chunk_matching(chunk_id) method added
+   - Explicitly tracks when matching starts
+
+### Priority 2: Important Enhancements
+
+4. **Add chunk state persistence**
+   - Persist ChunkHandle to F03 Flight Database
+   - Enable recovery after restart
+   - Support debugging and analysis
+
+5. ✅ **Clarify method naming** - **FIXED**
+   - F10.get_active_chunk() renamed to get_chunk_for_frame() for clarity
+   - Documented different use cases (low-level vs high-level)
+
+6. **Add chunk validation**
+   - Validate chunk state before operations
+   - Prevent invalid state transitions
+   - Better error messages
+
+### Priority 3: Nice-to-Have
+
+7. **Add chunk metrics**
+   - Track chunk creation time
+   - Track matching success rate
+   - Track merging statistics
+
+8. **Add chunk query methods**
+   - Query chunks by status
+   - Query chunks by frame range
+   - Query chunks by matching status
+
+---
+
+## 10. Overall Assessment
+
+### Coherence Score: 9/10 (Updated after fixes)
+
+**Strengths:**
+- ✅ Clear separation of concerns
+- ✅ Proactive chunk creation
+- ✅ Complete lifecycle coverage
+- ✅ Well-integrated with F10, F11, F02
+- ✅ Proper chunk isolation
+- ✅ **FIXED:** Merging coordination through F12
+- ✅ **FIXED:** Descriptor computation delegation
+- ✅ **FIXED:** Explicit state transitions
+
+**Remaining Enhancement:**
+- ⚠️ No state persistence (Priority 2, not critical)
+
+**Conclusion:** The chunking system is **excellent and coherent** with clear architectural boundaries. All Priority 1 issues have been **fixed**. The system now properly maintains abstraction layers with F12 coordinating all chunk lifecycle operations. Remaining enhancement (state persistence) is a nice-to-have feature for recovery and debugging.
+
+---
+
+## 11. Solution Alignment
+
+### Atlas Multi-Map Architecture ✅
+
+The chunking system correctly implements the Atlas architecture from the solution:
+
+- ✅ **Chunks are first-class entities** - F12 manages chunks as primary units
+- ✅ **Proactive chunk creation** - Chunks created immediately on tracking loss
+- ✅ **Independent chunk processing** - Each chunk has its own subgraph in F10
+- ✅ **Chunk matching and merging** - Semantic matching → LiteSAM → Sim(3) merging
+- ✅ **Multiple chunks simultaneously** - System supports multiple unanchored chunks
+
+**Assessment:** ✅ **Fully aligned with solution architecture**
+