Skip to content

Phase 6: Complete library/CLI separation and API finalization #33

Closed
#34
Closed
Phase 6: Complete library/CLI separation and API finalization#33
#34
Labels
documentationImprovements or additions to documentationenhancementNew feature or request
@coderabbitai

Description

@coderabbitai
coderabbitai
bot
opened on Dec 19, 2025

Summary

With PR #32 merged, issue #27 is now complete (library export integration tests added). However, to fully achieve the goals of de-duplication and functionality migration for dual CLI and CRATE usage, several important follow-up items remain.

This issue outlines a detailed iterative plan to complete the library/CLI separation and ensure all crate-consumer-useful functionality is properly exposed.

Context

Current Achievement Status:

Architecture Baseline:

  • src/lib.rs: Public API with re-exports from all modules
  • src/main.rs: CLI entry point (~1405 lines, 15 functions)
  • Export functions (export_to_csv, export_to_gpx, export_to_event) are in library
  • CLI uses library functions with CLI-specific status messages

Retrieved Learning:

Decision criteria: "Is this needed by crate consumers?" determines placement — shared logic in library, CLI-only logic in src/main.rs

Remaining Work

Phase 6: Finalize Library API and Eliminate Remaining Duplication

Item 1: Audit and resolve parsing function duplication ⚠️ HIGH PRIORITY

Problem: src/main.rs contains functions named parse_bbl_file() and parse_single_log() that may duplicate or diverge from library parsing functions.

Investigation needed:

  • Compare main.rs::parse_bbl_file() (line 481) with library's parse_bbl_file()
  • Check if main.rs::parse_single_log() (line 554) is CLI-specific or should be in library
  • Verify main.rs::parse_bbl_file_streaming() (line 1004) - streaming API for library?

Expected outcome:

  • CLI uses library parsing functions exclusively
  • No duplicate parsing logic in main.rs
  • If streaming API is valuable for crate consumers, expose it in library
  • Document any CLI-specific parsing wrappers and their purpose

Acceptance criteria:

  • No function name collisions between main.rs and library
  • main.rs imports and uses library parsing functions
  • Streaming export capability is properly exposed (if library-worthy)

Item 2: Evaluate export heuristics for library inclusion 🔍 MEDIUM PRIORITY

Problem: src/main.rs contains export skip/filter heuristics that might be useful for library consumers:

  • should_skip_export() (line 854) - skips very short flights or minimal gyro activity
  • has_minimal_gyro_activity() (line 929) - detects ground tests
  • calculate_variance() (line 993) - statistical helper

Question: Should these be:

  1. CLI-only features (stay in main.rs), OR
  2. Optional library features via ExportOptions?

Investigation needed:

  • Would crate consumers benefit from configurable export filtering?
  • Should ExportOptions gain fields like skip_short_flights: bool, min_duration_ms: Option<u64>, skip_ground_tests: bool?
  • Consider privacy: some users may want ALL data, others may want filtered exports

Proposed solution:

  • Add optional filter settings to ExportOptions with sensible defaults
  • Move heuristic functions to library (e.g., src/export.rs or new src/filters.rs)
  • CLI enables filters by default; library consumers can opt in/out
  • Keep CLI-specific messaging in main.rs

Acceptance criteria:

  • Export filtering is either explicitly marked CLI-only OR moved to library with proper configuration
  • No silent filtering behavior surprises library consumers
  • CLI maintains current filtering behavior via library options

Item 3: Implement ExportReport return type (from issue #27, Item #5) 📋 MEDIUM PRIORITY

Problem: Export functions currently return Result<()>, making it impossible for callers to know which files were actually created.

Proposed enhancement:

pub struct ExportReport {
    pub csv_path: Option<PathBuf>,
    pub headers_path: Option<PathBuf>,
    pub gpx_path: Option<PathBuf>,
    pub event_path: Option<PathBuf>,
}

pub fn export_to_csv(...) -> Result<ExportReport> { ... }
pub fn export_to_gpx(...) -> Result<ExportReport> { ... }
pub fn export_to_event(...) -> Result<ExportReport> { ... }

Benefits:

  • Library consumers can log exact paths written
  • CLI can print precise export messages without recomputing paths
  • Enables post-export validation and testing
  • Reduces coupling between path computation and CLI messaging

Implementation plan:

  1. Define ExportReport struct in src/export.rs
  2. Update export functions to return Result<ExportReport>
  3. Update CLI to use returned paths for status messages
  4. Update integration tests to validate returned paths
  5. Add documentation examples

Acceptance criteria:

  • Export functions return actual paths written
  • CLI prints exact paths from ExportReport
  • Integration tests verify ExportReport accuracy
  • Breaking change documented in CHANGELOG

Item 4: Early-return cleanup (from issue #27, Item #3) 🔧 LOW PRIORITY

Current state: Export functions have early-returns for empty data (e.g., gps_coordinates.is_empty()), but path computation happens before the check.

Proposed cleanup:

  • Move early-return checks to the very top of export functions
  • Avoid computing paths when no export will happen
  • Ensure CLI messaging still works correctly

Implementation:

pub fn export_to_gpx(...) -> Result<ExportReport> {
    // Early return BEFORE path computation
    if gps_coordinates.is_empty() {
        return Ok(ExportReport::default());
    }
    
    // Now compute paths only if exporting
    let paths = compute_export_paths(...);
    // ... rest of export logic
}

Acceptance criteria:

  • Early-returns happen before path computation
  • CLI messages remain correct (use compute_export_paths() for display)
  • No performance impact on actual exports
  • Integration tests pass

Item 5: Public API audit and refinement 🏗️ MEDIUM PRIORITY

Problem: src/lib.rs currently re-exports everything with pub use module::*, which may expose internal implementation details.

Investigation needed:

  • Review what should be public vs. internal
  • Check for unintentional public API surface (e.g., internal helpers)
  • Verify consistency with documented public API in learnings:

    Public API must expose: parse_bbl_file(), parse_bbl_bytes(), BBLLog, ExportOptions, export_to_csv(), export_to_gpx(), export_to_event(), conversion utilities, parser helpers

Proposed changes:

  • Replace wildcard re-exports with explicit item lists
  • Mark internal helpers with pub(crate) visibility
  • Document public API in src/lib.rs with examples
  • Consider creating a prelude module for common imports

Example refinement:

// Instead of:
pub use export::*;

// Use explicit exports:
pub use export::{
    ExportOptions, ExportReport,
    export_to_csv, export_to_gpx, export_to_event,
    compute_export_paths,
};

Acceptance criteria:

  • Only intended public items are exported
  • Internal helpers use pub(crate) or private visibility
  • Documentation clearly lists the public API
  • No breaking changes for existing consumers (if possible)

Item 6: Documentation completeness 📚 MEDIUM PRIORITY

Current state: Documentation exists in README.md, CRATE_USAGE.md, and examples/README.md, recently enhanced by PR #32.

Additional needs:

  • API reference documentation in rustdoc comments
  • Ensure all public functions have doc comments with examples
  • Document feature flags and their impact on API
  • Add crate-level documentation to src/lib.rs

Tasks:

  1. Add //! module-level docs to src/lib.rs with:
    • Feature flag table
    • Quick start example
    • Link to examples and CRATE_USAGE.md
  2. Ensure every public function has /// doc comments
  3. Add doc examples that are tested with cargo test --doc
  4. Document error types and recovery strategies

Acceptance criteria:

  • cargo doc --open produces comprehensive API documentation
  • All public items have rustdoc comments
  • Doc examples compile and run successfully
  • Feature flags are clearly documented

Item 7: CLI reduction and simplification 🔨 LOW PRIORITY

Current state: CLI is ~1405 lines (down from ~1800 after unification).

Opportunities:

  • If Items 1-2 move more logic to library, CLI may shrink further
  • Simplify CLI functions to thin wrappers around library calls
  • Review if any CLI helper functions have general utility

Investigation needed:

  • Can expand_input_paths() (glob/directory handling) be useful in library?
  • Are display functions (display_frame_data, display_debug_info) properly isolated?
  • Should should_have_frame() (line 250) be in library?

Proposed approach:

  1. After Items 1-2 complete, audit remaining main.rs functions
  2. Move any general-purpose helpers to library
  3. Keep pure CLI concerns (arg parsing, output formatting) in main.rs
  4. Target: CLI under 1000 lines if possible

Acceptance criteria:

  • CLI is minimal wrapper around library
  • No general-purpose logic remains CLI-only
  • CLI tests remain comprehensive
  • CLI behavior unchanged for end users

Implementation Order

Recommended iteration sequence:

  1. Iteration 1 (Critical): Item 1 - Resolve parsing duplication

    • Ensures foundation is solid before other changes
    • Prevents divergent implementations

  2. Iteration 2 (Enhancement): Item 3 - Implement ExportReport

    • Provides better API before Item 4 cleanup
    • Enables better testing and validation

  3. Iteration 3 (Cleanup): Item 4 - Early-return cleanup

    • Builds on Item 3's ExportReport structure
    • Minor polish with low risk

  4. Iteration 4 (Features): Item 2 - Export heuristics evaluation

    • After core API is stable (Items 1, 3, 4)
    • Decision-heavy; may need community input

  5. Iteration 5 (Polish): Item 5 - Public API audit

    • After all functionality is in place
    • Ensures clean API surface

  6. Iteration 6 (Documentation): Item 6 - Documentation completeness

    • After API is finalized
    • Prepares for stable release

  7. Iteration 7 (Optimization): Item 7 - CLI reduction

    • Final cleanup after all library work
    • Non-critical quality improvement

Testing Strategy

For each iteration:

  • Run full test suite: cargo test --verbose
  • Run CLI tests: cargo test --features=cli --verbose
  • Run integration tests: cargo test --test export_integration_tests
  • Verify Clippy: cargo clippy --all-targets --all-features -- -D warnings
  • Check formatting: cargo fmt --all -- --check
  • Build release: cargo build --release

Success Criteria

Overall goals achieved when:

  • ✅ No parsing logic duplication between CLI and library
  • ✅ All crate-consumer-useful functionality is in library
  • ✅ CLI is minimal wrapper with only CLI-specific logic
  • ✅ Export functions return actionable results (ExportReport)
  • ✅ Public API is clean, well-documented, and intentional
  • ✅ Export filtering is either explicitly CLI-only or library-configurable
  • ✅ Comprehensive documentation in rustdoc and examples
  • ✅ All tests pass across platforms
  • ✅ No clippy warnings or formatting issues

Related Issues

Notes

  • This is an iterative plan - each item can be addressed in a separate PR
  • Items can be reordered based on priority and dependencies
  • Some items may be marked "CLI-only" after discussion rather than moved to library
  • Breaking changes (e.g., ExportReport) should be versioned appropriately
  • Community feedback welcome on Item 2 (export heuristics) and Item 5 (public API)

Requested by: @nerdCopter
Date: 2025-12-19

Metadata

Metadata

Assignees

No one assigned

    Labels

    documentationImprovements or additions to documentationenhancementNew feature or request

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions