refactor: address PR #34 nitpick comments in filters module

[nerdCopter]

Addresses code review feedback for PR #34:

Changes:

1. Use BBLLog::duration_us() method instead of duplicate computation
2. Fix misleading comment on variance threshold logic
3. Add comprehensive unit tests for filtering heuristics (9 tests covering edge cases)

Tests:

• All 51+ tests pass
• Clippy: no warnings
• Formatting: clean

Follows up on PR #34 (feat: unify parsing, export, and filtering API) which was merged to master.

Summary by CodeRabbit

• Refactor

  • Updated flight duration calculations to use a centralized method instead of manual computation, improving consistency across filtering logic.
  • Clarified comments in gyro activity detection.

• Tests

  • Added comprehensive unit tests for flight filtering scenarios, including very short flights, density-based decisions, gyro activity assessment, and edge cases.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

The change refactors duration calculation logic in src/filters.rs to use a centralized duration_us() method instead of manual computation, and adds comprehensive unit tests validating filtering behavior across various scenarios.

Changes

Cohort / File(s)

Change Summary

Duration refactoring and unit tests src/filters.rs

Replaced manual duration calculations with log.duration_us() calls; updated conditional logic for short/very short flight handling to use computed duration and fps for density checks; adjusted comment wording for gyro activity variance explanation; added extensive #[cfg(test)] module with tests covering skip behavior, density-based decisions, force-export override, fallback logic, and variance calculation edge cases.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

• Verify that the refactored duration_us() calls produce identical behavior to the original manual calculations
• Validate that all filtering conditions (very short flights, density checks, gyro activity, frame count fallback) work correctly with the centralized duration source
• Confirm unit test coverage adequately validates the core filtering logic and edge cases

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'refactor: address PR #34 nitpick comments in filters module' accurately describes the main change—a refactoring that addresses specific code review feedback from PR #34 in the filters module.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch nitpick-fixes

---

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fb3f39c and 900de3a.

📒 Files selected for processing (1)

• src/filters.rs (3 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

src/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.rs: Do not remove or modify comments unless the related code is changed
Only add comments that explain code functionality; no AI instructional comments
Never embed or call external binaries from Rust code
Ensure cargo build --release has no errors or warnings
Only commit if cargo clippy --all-targets --all-features -- -D warnings passes
Only commit if cargo fmt --all -- --check passes
Only commit if cargo test --verbose passes
Only commit if cargo build --release passes with no errors or warnings
BEFORE ANY CODE CHANGES: Always run cargo clippy --all-targets --all-features -- -D warnings to catch ALL issues
BEFORE ANY CODE CHANGES: Always run cargo fmt --all -- --check to ensure formatting compliance
If cargo fmt --all -- --check fails, IMMEDIATELY run cargo fmt --all to fix formatting
Code must pass cargo fmt --all -- --check without any formatting issues
Never skip clippy checks or formatting checks. Never allow warnings to pass
ALWAYS run cargo fmt --all after making ANY code changes before moving to next steps
After running cargo fmt --all, ALWAYS verify with cargo fmt --all -- --check before proceeding

Files:

• src/filters.rs

{src/**/*.rs,Cargo.toml}

📄 CodeRabbit inference engine (AGENTS.md)

{src/**/*.rs,Cargo.toml}: Only commit if cargo test --features=cli --verbose passes
All feature combinations must compile without errors

Files:

• src/filters.rs

🧠 Learnings (3)

📓 Common learnings

Learnt from: nerdCopter
Repo: nerdCopter/bbl_parser PR: 0
File: :0-0
Timestamp: 2025-12-08T21:15:02.434Z
Learning: For bbl_parser reviews: nerdCopter prefers compact, single-block “outside diff” comments without nested <details> or admonitions; use flat bullets with file:line references and link a gist for overflow.

Learnt from: CR
Repo: nerdCopter/bbl_parser PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-13T15:51:00.137Z
Learning: Applies to {src/main.rs,src/conversion.rs,src/parser/{stream,helpers}.rs} : Comprehensive tests distributed across `src/main.rs`, `src/conversion.rs`, `src/parser/stream.rs`, and `src/parser/helpers.rs`

Learnt from: nerdCopter
Repo: nerdCopter/bbl_parser PR: 2
File: LICENSE_COMMERCIAL:1-4
Timestamp: 2025-08-29T19:52:05.099Z
Learning: nerdCopter prefers to avoid publishing personal information in license files for privacy and security reasons, as they are an individual maintainer rather than a company.

Learnt from: nerdCopter
Repo: nerdCopter/bbl_parser PR: 2
File: README.md:520-521
Timestamp: 2025-08-29T19:53:41.354Z
Learning: nerdCopter uses AGPL-3.0-or-later licensing for the bbl_parser project with a dual-licensing approach that includes a separate commercial license option.

Learnt from: nerdCopter
Repo: nerdCopter/bbl_parser PR: 2
File: CONTRIBUTING.md:9-14
Timestamp: 2025-08-21T20:25:45.741Z
Learning: nerdCopter prefers to keep CLA language general using "project maintainer" rather than specifying a legal entity name, as they are an individual maintainer without an associated company.

Learnt from: nerdCopter
Repo: nerdCopter/bbl_parser PR: 2
File: LICENSE_COMMERCIAL:0-0
Timestamp: 2025-08-29T20:15:04.624Z
Learning: nerdCopter prefers clear positive indicators (✅) when describing fixes rather than using ❌ symbols which can be confusing when describing what was corrected in a positive context.

📚 Learning: 2025-12-13T15:51:00.137Z

Learnt from: CR
Repo: nerdCopter/bbl_parser PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-13T15:51:00.137Z
Learning: Applies to src/lib.rs : 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

Applied to files:

• src/filters.rs

📚 Learning: 2025-12-13T15:51:00.137Z

Learnt from: CR
Repo: nerdCopter/bbl_parser PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-13T15:51:00.137Z
Learning: Applies to {src/main.rs,src/conversion.rs,src/parser/{stream,helpers}.rs} : Comprehensive tests distributed across `src/main.rs`, `src/conversion.rs`, `src/parser/stream.rs`, and `src/parser/helpers.rs`

Applied to files:

• src/filters.rs

🧬 Code graph analysis (1)

src/filters.rs (1)

src/types/log.rs (1)

• duration_us (41-45)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: Build Release Binaries (macos-latest, bbl_parser-macos-x64, target/release/bbl_parser)
• GitHub Check: Build Release Binaries (windows-latest, bbl_parser-windows-x64, target/release/bbl_parser.exe)
• GitHub Check: Build Release Binaries (ubuntu-latest, bbl_parser-linux-x64, target/release/bbl_parser)

🔇 Additional comments (3)

src/filters.rs (3)

36-37: Excellent refactoring to centralized duration calculation.

Using log.duration_us() eliminates code duplication and leverages the built-in saturating subtraction for safety. This makes the logic more maintainable and consistent across the codebase.

---

168-169: Comment clarification improves code readability.

The updated wording accurately describes that the highest variance across all axes is used, making the logic clearer for future maintainers.

---

193-304: Comprehensive test coverage for filtering heuristics.

The test suite thoroughly validates the filtering logic with excellent coverage of edge cases:

• Boundary conditions (5s, 15s, 1500fps, 7500 frames)
• Force export override behavior
• Fallback logic when duration is unavailable
• Variance calculation edge cases (empty, single value)

Well-structured tests with descriptive names that make the expected behavior clear.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}