feat: Physics-based optimal P estimation with prop-size-aware recommendations

[nerdCopter]

test-binaries. commit 84a56e2
binary-windows.zip
binary-macos.zip
binary-linux.zip

---

TL;DR: "generates/analyzes step response, then compares that to physics-based targets per prop-size. targets are empirical values that need statistical research still."

---

Overview

Adds physics-aware optimal P gain estimation based on Brian White's (PIDtoolbox author @bw1129) theory: optimal response timing is aircraft-specific, determined by power-to-rotational-inertia ratio.

What This Does

Analyzes your flight logs and recommends optimal P gains (and corresponding D values) based on:

• Response time (Td): How quickly your quad reaches 50% of setpoint
• Noise levels: Detects if you're hitting thermal/mechanical limits
• Consistency: Warns if measurements are unreliable

Testing Instructions

Basic Usage (generates step response plots only):

./target/release/BlackBox_CSV_Render ./input/*.csv --step --estimate-optimal-p --prop-size 5 --output-dir ./output

Important: Use --step flag to skip other graphs (faster testing). The feature only affects step response analysis.

Prop Size Selection:

• Match your actual propeller diameter, not frame size
• Example: 6" frame with 5" props → use --prop-size 5
• Example: 7" frame with 6" props → use --prop-size 6
• Supported: 1-15 inches

What to Look For

Console Output:

Roll: Td=18.3ms (target 20.0±5.0ms, -8% dev), Noise=LOW, Consistency=79%
  Current P=53
    → Increase recommended:
      Conservative: P≈54 (+1), D≈45 (+3)
    Response is faster than target with low noise levels...

PNG Legend (appears on step response plots):

Optimal P (5")
  Td: 18.3ms (target: 20.0ms)
  Deviation: -8% (WITHIN TARGET)
  Noise: LOW
  Recommendation: P≈54 (+1), D≈45 (+3)

Warning Example:

[WARNING] High variability (CV=44%) - results may be unreliable

This means your stick inputs were inconsistent. Fly cleaner flips/rolls for better measurements.

Key Features

1. Prop-size-aware targets (1"-13" props supported)

   • Each prop size has physics-based response time targets
   • Automatically scales: smaller props = faster response, larger props = slower

2. Complete tuning guidance

   • Recommends both P and D values together
   • Uses recommended P:D ratio from step response analysis
   • Conservative tier only

3. Noise analysis

   • LOW noise = headroom to increase P
   • HIGH noise = at thermal/mechanical limits
   • Helps avoid motor overheating

4. Quality control

   • Warns if measurements are unreliable
   • Shows consistency metrics
   • Helps identify mechanical issues

What to Test

1. Accuracy: Do recommendations make sense for your quad?
2. Prop size logic: Test with different prop sizes (if you have multiple quads)
3. Edge cases: Very old logs, missing data, unusual configurations
4. Clarity: Are recommendations easy to understand?
5. PNG readability: Is the legend information useful?

Changes vs. Main

• No breaking changes - existing features unchanged
• New --estimate-optimal-p flag (optional, off by default)
• New --prop-size <size> flag (only used with optimal-P)
• Step response plots enhanced with optional optimal-P legend
• Console output adds optional optimal-P analysis section

Documentation

See OVERVIEW.md for:

• Theory explanation
• Validation methodology
• Target values for each prop size
• Physics background (I ∝ r²)

Known Limitations

• Targets are provisional (need more flight data validation)
• Works best with clean stick inputs (quick flips/rolls)
• Requires D-term data for noise analysis
• Yaw axis not analyzed (intentionally - different dynamics)

Feedback Wanted

• Do P/D recommendations improve flight performance?
• Are prop-size targets accurate for your setup?
• Is the output clear and actionable?
• Any confusing warnings or terminology?
• PNG legend layout improvements?

---

*Testing this branch helps validate the physics theory with real-world data 🚁

Summary by CodeRabbit

• New Features

  • Optional Optimal P Estimation: prop-size (1–15") frame-class targets, Td statistics, high-frequency noise metric, and actionable P recommendations with console reports.

• Plots

  • Step-response plots can overlay Optimal P analysis summaries and recommendation labels per axis.

• Documentation

  • Expanded README and overview: new grouped CLI help, --estimate-optimal-p/--prop-size flags, usage examples, and console output descriptions.

• Tests

  • Unit tests added to validate frame-size Td target lookup and expected targets.

[coderabbitai]

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 63362db9-45f0-47cc-a422-77ac3cd2c098

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds an experimental Optimal P Estimation feature: new CLI flags (--estimate-optimal-p, --prop-size), frame-size Td targets (1–15"), Td statistics and consistency checks, HF D-term energy metric, decision logic producing P recommendations, integration into processing and plotting, supporting constants, spectral helper, docs, and unit tests.

Changes

Cohort / File(s)	Summary
Documentation OVERVIEW.md, README.md	New "Optimal P Estimation" docs and examples; CLI help reorganized into option groups; added --estimate-optimal-p and --prop-size flags; console output extended with prop-size-aware Td and recommendation metrics.
Constants & Targets src/constants.rs	Added public TdTargetSpec, TD_TARGETS (1–15"), lookup helper, and many public constants for D-term HF thresholds, Td consistency/CV limits, P headroom/reduction multipliers, Td stat thresholds, and data collection minima.
Data analysis module src/data_analysis/mod.rs, src/data_analysis/optimal_p_estimation.rs	New optimal_p_estimation module with FrameClass (1–15"), NoiseLevel, TdDeviation, PRecommendation, TdStatistics, OptimalPAnalysis (analyze/generate_recommendation/format_console_output), and AnalysisError.
Spectral helper src/data_analysis/spectral_analysis.rs	Added pub fn calculate_hf_energy_ratio(...) -> Option<f64> using Welch PSD to compute HF-energy ratio (duplicate declaration observed in diff).
Main & CLI wiring src/main.rs	Introduced internal AnalysisOptions, consolidated analysis flags, added/validated --estimate-optimal-p and --prop-size, updated process_file signature, runs Optimal P analysis per-axis and stores results for plotting.
Plotting integration src/plot_functions/plot_step_response.rs	Refactored plot_step_response API to accept new structs (PdRecommendations, ConservativeRecommendations, ModerateRecommendations, CurrentPeakAndRatios, PlotDisplayConfig, OptimalPConfig) and integrated per-axis OptimalPAnalysis output into plot annotations and legend.
Axis constants src/axis_names.rs	Simplified axis_name lookup, added pub const ROLL_PITCH_AXIS_COUNT: usize = 2, and a compile-time bound assertion.
Tests src/data_analysis/tests_optimal_p.rs	New unit tests for Td target lookup and FrameClass ↔ TdTargetSpec value consistency and out-of-range behavior.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• feat: Bode plot analysis for transfer function visualization #118 — Adds PSD/Welch utilities and PSD_EPSILON; directly related to the HF-energy ratio computation used by optimal-p analysis.
• feat: advanced step response analysis, dual P:D recommendations, D-Min/D-Max support, and robust safety features #69 — Overlaps step-response analysis and plotting integration; likely touches the same plotting wiring and recommendation rendering.
• feat: improve plot titles to use axis names (Roll, Pitch, Yaw) instead of index #42 — Modifies plot_step_response.rs (signature/labeling) and may conflict with the refactor introducing new plotting structs and outputs.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title directly and clearly summarizes the main feature being added: a physics-based optimal P estimation system that uses prop-size-aware recommendations. It accurately reflects the primary changeset across documentation, CLI, constants, and core analysis logic.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feature/optimal-p-estimation

---

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.}

[coderabbitai]

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

OVERVIEW.md (1)

3-15: Update Table of Contents for the new section.

As per coding guidelines, the Table of Contents should be updated when adding new sections. The new "Optimal P Estimation (Optional)" section at line 192 should be added to the TOC.

Proposed TOC addition

   - [Compared to PIDtoolbox/Matlab (PTstepcalc.m)](`#compared-to-pidtoolboxmatlab-ptstepcalcm`)
   - [Compared to PlasmaTree/Python (PID-Analyzer.py)](`#compared-to-plasmatreepython-pid-analyzerpy`)
+  - [Optimal P Estimation (Optional)](`#optimal-p-estimation-optional`)

Add this entry in the appropriate location within the TOC hierarchy (under the output/tuning section or as a top-level entry).

🤖 Fix all issues with AI agents

In `@README.md`:
- Line 29: The README usage line is inconsistent with the actual CLI flag
rename: replace the incorrect flag `--frame-class <size>` with the correct
`--prop-size <size>` so the Usage string matches the help text, example and the
flag defined in main.rs (ensure any other occurrences of `--frame-class` in
README are updated to `--prop-size` for consistency with the CLI).
- Around line 113-116: Change the terminology "Frame-class-aware Td" to
"Prop-size-aware Td" in the README to match the renamed CLI flag and PR
language; specifically update the section describing "Optimal P estimation (when
--estimate-optimal-p is used)" so the bulleted line reads "Prop-size-aware Td
(time to 50%) analysis" and scan nearby documentation for any other occurrences
of "Frame-class-aware" to replace with "Prop-size-aware" to keep terminology
consistent with the --prop-size flag and the PR title "prop-size-aware
recommendations".

In `@src/data_analysis/optimal_p_estimation.rs`:
- Line 286: The current mapping let current_pd_ratio = current_d.map(|d|
(current_p as f64) / (d as f64)); can divide by zero when current_d is Some(0);
change the mapping to guard against d == 0 by using and_then (or map +
conditional) so that when d == 0 you return None instead of computing an
infinite ratio, e.g. replace map with current_d.and_then(|d| if d == 0 { None }
else { Some((current_p as f64) / (d as f64)) }) to keep current_pd_ratio as
Option<f64>.
- Around line 583-591: Guard the D-calculation against rec_pd being zero before
dividing: in the block that checks (self.current_d,
self.recommended_pd_conservative) and computes conservative_d using
((*conservative_p as f64) / rec_pd).round() as u32, first test if rec_pd == 0.0
(or rec_pd.is_normal()) and if so skip adding the D estimate (or append a
sensible placeholder like "D≈N/A") instead of performing the division; apply the
same guard to the other two places where you compute D using
recommended_pd_conservative/recommended_pd_aggressive (the analogous blocks that
compute conservative_d/aggressive_d and their deltas) so no division by zero can
produce f64::INFINITY and u32::MAX.

In `@src/main.rs`:
- Around line 415-421: Update the example usage string in main.rs where the
eprintln! examples are printed: replace the outdated flag name "--frame-class"
with the current flag "--prop-size" in the eprintln! call that contains
"{program_name} flight.csv --step --estimate-optimal-p --frame-class 5" so the
example matches the actual CLI flag; keep the rest of the eprintln! messages
unchanged.

🧹 Nitpick comments (2)

src/data_analysis/optimal_p_estimation.rs (2)

188-218: Move MEAN_EPSILON to src/constants.rs.

Per coding guidelines, all constants should be in src/constants.rs — no hardcoded magic numbers in function code. The local constant MEAN_EPSILON should be moved to the constants module.

Suggested refactor

In src/constants.rs, add:

/// Epsilon threshold for near-zero mean values in Td statistics
pub const TD_MEAN_EPSILON: f64 = 1e-12;

Then update this file:

     pub fn from_samples(td_samples_ms: &[f64]) -> Option<Self> {
-        const MEAN_EPSILON: f64 = 1e-12; // Threshold for near-zero mean values
-
         if td_samples_ms.is_empty() {
             return None;
         }
         // ...
-        if mean.abs() <= MEAN_EPSILON {
+        if mean.abs() <= TD_MEAN_EPSILON {
             return None;
         }

---

294-303: Move deviation threshold constants to src/constants.rs.

Per coding guidelines, the magic numbers 30.0, 15.0, and -15.0 for deviation classification should be defined as named constants in the constants module.

Suggested refactor

In src/constants.rs, add:

/// Td deviation thresholds for P recommendation classification (percentages)
pub const TD_DEVIATION_SIGNIFICANTLY_SLOWER_THRESHOLD: f64 = 30.0;
pub const TD_DEVIATION_MODERATELY_SLOWER_THRESHOLD: f64 = 15.0;
pub const TD_DEVIATION_SIGNIFICANTLY_FASTER_THRESHOLD: f64 = -15.0;

Then update the classification logic:

         // Classify deviation
-        let td_deviation = if td_deviation_percent > 30.0 {
+        let td_deviation = if td_deviation_percent > TD_DEVIATION_SIGNIFICANTLY_SLOWER_THRESHOLD {
             TdDeviation::SignificantlySlower
-        } else if td_deviation_percent > 15.0 {
+        } else if td_deviation_percent > TD_DEVIATION_MODERATELY_SLOWER_THRESHOLD {
             TdDeviation::ModeratelySlower
-        } else if td_deviation_percent < -15.0 {
+        } else if td_deviation_percent < TD_DEVIATION_SIGNIFICANTLY_FASTER_THRESHOLD {
             TdDeviation::SignificantlyFaster
         } else {
             TdDeviation::WithinTarget
         };

[coderabbitai]

Actionable comments posted: 3

🤖 Fix all issues with AI agents

In `@src/data_analysis/optimal_p_estimation.rs`:
- Around line 191-299: Extract the hard-coded numeric constants into
src/constants.rs (e.g., TD_MEAN_EPSILON, TD_DEVIATION_SIGNIFICANT_PERCENT,
TD_DEVIATION_MODERATE_PERCENT) and replace usages: swap the local MEAN_EPSILON
inside TdStatistics::from_samples with TD_MEAN_EPSILON, and replace the
30.0/15.0/-15.0 deviation literals in OptimalPAnalysis::analyze (the
td_deviation_percent comparisons) with TD_DEVIATION_SIGNIFICANT_PERCENT and
TD_DEVIATION_MODERATE_PERCENT (derive the negative cutoff from the moderate
constant or add a TD_DEVIATION_NEGATIVE_MODERATE_PERCENT if preferred); add the
necessary use/import of the constants module where TdStatistics::from_samples
and OptimalPAnalysis::analyze are defined. Ensure names are clear and update any
documentation comments accordingly.

In `@src/main.rs`:
- Around line 352-356: The usage string in print_usage_and_exit still references
the removed flag `--frame-class`, causing incorrect help output; update the
usage synopsis inside the print_usage_and_exit function to replace
`--frame-class <size>` with the current flag `--prop-size <size>` so the CLI
help matches the parser and documentation (ensure the string interpolation for
program_name remains intact and adjust any surrounding spacing/line breaks to
keep formatting consistent).
- Around line 963-1056: Replace hard-coded numeric literals and manual axis
range with constants and axis helpers: move the 1000.0 millisecond multiplier
into src/constants.rs (e.g., TIME_MS_PER_S) and the minimum D-term sample cutoff
100 into a constant (e.g., DTERM_MIN_SAMPLES), then use those constants in
td_samples_ms conversion and the d_term_data.len() check; replace the raw axis
loop "for axis_index in 0..2" with the axis_names helper (use
crate::axis_names::AXIS_NAMES with enumerate()/iter().take() or the project’s
provided axis index helper) so the loop uses the canonical axis set; update
references in this block (td_samples_ms, axis_index,
calc_step_response::calculate_delay_time, and the d-term HF calculation call) to
use the new constants and helper.

♻️ Duplicate comments (3)

src/main.rs (1)

415-420: Example still uses the old flag name.

The example should use --prop-size instead of --frame-class.

🛠️ Suggested fix

-  {program_name} flight.csv --step --estimate-optimal-p --frame-class 5
+  {program_name} flight.csv --step --estimate-optimal-p --prop-size 5

src/data_analysis/optimal_p_estimation.rs (2)

282-283: Guard current_pd_ratio against D=0.

If current_d is Some(0), this yields inf. The ratio is stored and can leak into outputs.

🛠️ Suggested fix

- let current_pd_ratio = current_d.map(|d| (current_p as f64) / (d as f64));
+ let current_pd_ratio = current_d.and_then(|d| {
+     if d > 0 {
+         Some((current_p as f64) / (d as f64))
+     } else {
+         None
+     }
+ });

---

526-620: Guard D recommendations against rec_pd == 0.

If rec_pd is 0.0, the division yields inf and rounds to u32::MAX, producing nonsensical D values.

🛠️ Suggested fix

- if let (Some(current_d), Some(rec_pd)) =
-     (self.current_d, self.recommended_pd_conservative)
- {
-     let conservative_d = ((*conservative_p as f64) / rec_pd).round() as u32;
-     let conservative_d_delta = conservative_d as i32 - current_d as i32;
-     output.push_str(&format!(
-         ", D≈{} ({:+})",
-         conservative_d, conservative_d_delta
-     ));
- }
+ if let (Some(current_d), Some(rec_pd)) =
+     (self.current_d, self.recommended_pd_conservative)
+ {
+     if rec_pd > 0.0 {
+         let conservative_d = ((*conservative_p as f64) / rec_pd).round() as u32;
+         let conservative_d_delta = conservative_d as i32 - current_d as i32;
+         output.push_str(&format!(
+             ", D≈{} ({:+})",
+             conservative_d, conservative_d_delta
+         ));
+     }
+ }

[coderabbitai]

Actionable comments posted: 3

🤖 Fix all issues with AI agents

In `@src/constants.rs`:
- Around line 346-348: The constant OPTIMAL_P_MS_TO_SECONDS_MULTIPLIER is
misnamed (it actually multiplies seconds to produce milliseconds); rename it to
a clearer identifier such as OPTIMAL_P_SECONDS_TO_MS_MULTIPLIER or
OPTIMAL_P_SECONDS_TO_MILLIS_MULTIPLIER, update its declaration accordingly, and
update every call-site that references OPTIMAL_P_MS_TO_SECONDS_MULTIPLIER to use
the new name (search for the symbol and replace all occurrences to keep
compilation consistent).

In `@src/data_analysis/optimal_p_estimation.rs`:
- Around line 234-238: The is_consistent method currently only checks
consistency and coefficient_of_variation (using TD_CONSISTENCY_MIN_THRESHOLD and
TD_COEFFICIENT_OF_VARIATION_MAX) but treats single-sample Td sets as valid;
update is_consistent to first return false for any dataset with fewer than two
measurements (check the struct's sample count field or measurements.len()), and
only then apply the existing consistency and coefficient_of_variation thresholds
in is_consistent.

In `@src/main.rs`:
- Around line 1034-1038: The condition that gates D-term analysis uses a strict
greater-than, skipping analysis when d_term_data.len() equals the minimum
threshold; change the comparison in the if that references d_term_data and
crate::constants::OPTIMAL_P_MIN_DTERM_SAMPLES from ">" to ">=" so analysis runs
when the sample count is exactly the configured minimum (i.e., ensure the if
block that checks !d_term_data.is_empty() && d_term_data.len() >
crate::constants::OPTIMAL_P_MIN_DTERM_SAMPLES uses >=).

[coderabbitai]

Actionable comments posted: 3

🤖 Fix all issues with AI agents

In `@OVERVIEW.md`:
- Around line 192-265: Add a Table of Contents entry for the new "Optimal P
Estimation (Optional, Experimental)" section in OVERVIEW.md so the TOC includes
the new heading; locate the TOC block at the top of OVERVIEW.md and insert a
link pointing to the section header (use the exact header text "Optimal P
Estimation (Optional, Experimental)" as the anchor), and ensure the entry
mentions the related flags (--estimate-optimal-p, --prop-size) and the constants
reference (TD_TARGETS) so readers can find usage and the src/constants.rs
reference from the TOC.

In `@src/data_analysis/optimal_p_estimation.rs`:
- Around line 34-35: MIN_TD_MS is a domain constant that should be moved out of
optimal_p_estimation.rs into the shared constants module; remove the local const
MIN_TD_MS from optimal_p_estimation.rs, add a pub const MIN_TD_MS: f64 = 0.1; to
the central constants module, and update any references in
optimal_p_estimation.rs to refer to the new constant (e.g.,
crate::constants::MIN_TD_MS or by importing constants::MIN_TD_MS) so visibility
and usage remain correct.

In `@src/data_analysis/spectral_analysis.rs`:
- Around line 333-399: The function calculate_hf_energy_ratio contains a magic
numeric guard (1e-12); replace it with a shared constant PSD_EPSILON from the
project's constants module: add a public PSD_EPSILON (value 1e-12 or a named
value you prefer) to the constants module if it doesn't exist, import it into
src/data_analysis/spectral_analysis.rs (e.g., use crate::constants::PSD_EPSILON)
and change the final energy check from `if total_energy > 1e-12` to `if
total_energy > PSD_EPSILON`; ensure constants are exported publicly so the
function compiles.

🧹 Nitpick comments (2)

src/main.rs (1)

969-976: Move magic number 1000.0 to constants.

Line 975 uses a hardcoded 1000.0 for seconds-to-milliseconds conversion. Per coding guidelines, all constants should go in src/constants.rs.

♻️ Suggested fix

In src/constants.rs, add (if not already present):

/// Multiplier to convert seconds to milliseconds
pub const SECONDS_TO_MS: f64 = 1000.0;

Then update line 975:

-                                td_samples_ms.push(td_seconds * 1000.0);
+                                td_samples_ms.push(td_seconds * crate::constants::SECONDS_TO_MS);

As per coding guidelines, no hardcoded magic numbers in function code.

src/plot_functions/plot_step_response.rs (1)

19-20: Use AXIS_COUNT from axis_names module instead of defining locally.

This creates a redundant constant that duplicates the centralized axis system in src/axis_names.rs. Per coding guidelines, axis indices must use the axis_names module.

♻️ Suggested fix

+use crate::axis_names::AXIS_COUNT;
 use crate::axis_names::AXIS_NAMES;
 // ... other imports

-/// Number of control axes (Roll, Pitch, Yaw)
-const AXIS_COUNT: usize = 3;

The structs at lines 25-72 will then use the imported constant, ensuring consistency with the centralized axis naming system.

As per coding guidelines, axis-related constants must use the axis_names module.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (3)

README.md (1)

52-53: README mirrors main.rs help text—same clarity improvement applies.

The same bidirectional dependency clarification suggested for src/main.rs (lines 385-386) should be applied here to keep documentation synchronized.

Based on learnings: "Keep README.md usage/examples synchronized with actual CLI flags in src/main.rs"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@README.md` around lines 52 - 53, Update README.md to clarify the dependency
between the CLI flags by modifying the description lines for
--estimate-optimal-p and --prop-size: make it explicit that --prop-size only
takes effect when --estimate-optimal-p is also provided and that
--estimate-optimal-p may require a valid --prop-size value for certain
behaviors; mirror the same bidirectional wording used in src/main.rs (flags
--estimate-optimal-p and --prop-size) so the README stays synchronized with the
actual CLI help text.

src/main.rs (2)

1381-1386: Dead code: warning will never trigger.

The guard (1..=15).contains(&size) at line 1374 ensures size is in valid range before FrameClass::from_inches() is called. Since from_inches() returns Some for all values 1–15, frame_class_override.is_none() will always be false here.

This serves as defensive programming against future changes to from_inches(), which is reasonable. Consider adding a comment to clarify the intent:

+                        // Defensive check: from_inches() should never return None for validated 1-15 range
                         if frame_class_override.is_none() {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main.rs` around lines 1381 - 1386, The warning eprintln for
frame_class_override being None is dead code because
FrameClass::from_inches(size) currently returns Some for 1..=15; replace the
eprintln with a short clarifying comment (near the check that uses
frame_class_override and the call to FrameClass::from_inches) that documents
this is an intentional defensive check against future changes to from_inches,
referencing the variables/methods frame_class_override, from_inches, and size so
future readers understand why the None branch exists.

---

385-386: Help text understates the --prop-size requirement.

The help says --prop-size "Requires --estimate-optimal-p to have effect" but doesn't clarify that --estimate-optimal-p requires --prop-size to be specified (see validation at lines 1449-1456 which errors out if missing). Consider clarifying the bidirectional dependency:

📝 Suggested clarification

-    eprintln!("  --estimate-optimal-p: Enable optimal P estimation with frame-class targets.");
-    eprintln!("    --prop-size <size>: Propeller diameter in inches (1-15, whole-number only). Requires --estimate-optimal-p to have effect.");
+    eprintln!("  --estimate-optimal-p: Enable optimal P estimation with frame-class targets (requires --prop-size).");
+    eprintln!("    --prop-size <size>: Propeller diameter in inches (1-15, whole-number only). Required with --estimate-optimal-p.");

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main.rs` around lines 385 - 386, Update the help text printed in main
(the eprintln calls that emit the CLI usage lines for "--estimate-optimal-p" and
"--prop-size") to state the mutual dependency: that --prop-size requires
--estimate-optimal-p to have effect and --estimate-optimal-p also requires
--prop-size to be provided; mirror the exact validation logic used later (the
check that errors when --estimate-optimal-p is set without a --prop-size) so
users know both flags must be supplied together.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@README.md`:
- Line 106: The README claims "std dev" is shown but format_console_output()
actually prints Consistency percent and CV (with warnings) not standard
deviation; update the documentation to match actual output by removing "std dev"
from the list or change the function to also compute and display standard
deviation. Refer to format_console_output() and any Consistency/CV output text
when making the change so the README and console output remain consistent.

---

Nitpick comments:
In `@README.md`:
- Around line 52-53: Update README.md to clarify the dependency between the CLI
flags by modifying the description lines for --estimate-optimal-p and
--prop-size: make it explicit that --prop-size only takes effect when
--estimate-optimal-p is also provided and that --estimate-optimal-p may require
a valid --prop-size value for certain behaviors; mirror the same bidirectional
wording used in src/main.rs (flags --estimate-optimal-p and --prop-size) so the
README stays synchronized with the actual CLI help text.

In `@src/main.rs`:
- Around line 1381-1386: The warning eprintln for frame_class_override being
None is dead code because FrameClass::from_inches(size) currently returns Some
for 1..=15; replace the eprintln with a short clarifying comment (near the check
that uses frame_class_override and the call to FrameClass::from_inches) that
documents this is an intentional defensive check against future changes to
from_inches, referencing the variables/methods frame_class_override,
from_inches, and size so future readers understand why the None branch exists.
- Around line 385-386: Update the help text printed in main (the eprintln calls
that emit the CLI usage lines for "--estimate-optimal-p" and "--prop-size") to
state the mutual dependency: that --prop-size requires --estimate-optimal-p to
have effect and --estimate-optimal-p also requires --prop-size to be provided;
mirror the exact validation logic used later (the check that errors when
--estimate-optimal-p is set without a --prop-size) so users know both flags must
be supplied together.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0ce9b82b-29d7-43f7-b720-cd0c237c8905

📥 Commits

Reviewing files that changed from the base of the PR and between 84a56e2 and cc34292.

📒 Files selected for processing (2)

• README.md
• src/main.rs

[bw1129]

looks interesting! Thanks for referencing me

[nerdCopter]

Thoughts:

• I don't like the dependency on empirical thrust-targets table per propeller size.
• too many variables per aircraft.

What if:

• we calculate thrust of a given aircraft (via Blackbox as that is already how I (AI) calculated my 5-in value).
• i presume we need full-sweep throttle logs.
• what are an effective X quantity of logs? Can it work with 1? I presume multiple logs would be beneficial but how many are minimally effective?

Logistics:

• how to calculate the thrust given a set of logs? do we iterate each log calculate thrust in totality then reiterate each log for the step response estimated optimal p? Or would we calculate thrust per log and average them? Then perform the step response estimated optimal p?
• how to logistically wrap this in the code base? Would this also entail grouping Blackbox logs via similar file names (aircraft specific logs)?

@coderabbitai, review this comment and offer recommendations based on this branch's codebase. I'm willing to fork or re-implement.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== calc_step_response.rs structure ==="
cat src/data_analysis/calc_step_response.rs | head -120

echo -e "\n=== optimal_p_estimation.rs - analyze function ==="
rg -n "pub fn analyze|fn generate_recommendation|td_samples|physics_td_target|TdStatistics::from_samples" src/data_analysis/optimal_p_estimation.rs -A 3

echo -e "\n=== main.rs - optimal P block ==="
rg -n "estimate_optimal_p|td_samples|hf_energy|OptimalPAnalysis|FrameClass|physics" src/main.rs -A 3 | head -120

echo -e "\n=== What Blackbox fields are parsed? ==="
rg -n "setpoint|rcCommand|motor|axisP|axisD|gyro|loopIteration|time" src/data_parsing/ --type=rs | head -80

echo -e "\n=== Step response Td calculation ==="
rg -n "time_to_50|td_|Td|half_rise|step_response_windows|valid_windows" src/data_analysis/calc_step_response.rs | head -60

Length of output: 12102

---

🏁 Script executed:

#!/bin/bash
echo "=== Td calculation function (line 452+) ==="
sed -n '450,520p' src/data_analysis/calc_step_response.rs

echo -e "\n=== Data parsing - what fields are available ==="
fd -e rs . src/data_parsing/ | xargs grep -n "motor\|rcCommand\|setpoint\|throttle\|vbat\|amperage\|axisP\|axisD\|gyro" | head -80

echo -e "\n=== Types/structs for parsed log data ==="
cat src/types.rs | head -100

echo -e "\n=== main.rs - how all_log_data is structured ==="
rg -n "all_log_data|LogEntry\|DataRow\|parsed_data\|motor\[" src/main.rs | head -40

echo -e "\n=== check for throttle/motor commands in data pipeline ==="
rg -rn "motor\[0\]\|rcCommand\[3\]\|throttle\|MotorOutput" src/ --include="*.rs" | grep -v "target/" | head -40

Length of output: 8133

---

🏁 Script executed:

#!/bin/bash
echo "=== LogRowData struct ==="
fd log_data.rs src/ | xargs cat

echo -e "\n=== throttle heatmap - how throttle is accessed ==="
fd plot_throttle_freq_heatmap.rs src/ | xargs cat | head -60

echo -e "\n=== motor spectrum - how motor data is accessed ==="
fd -e rs . src/ | xargs grep -l "motor" | head -5 | while read f; do
  echo "--- $f ---"
  grep -n "motor\|\.motor\|rcCommand\[3\]\|throttle" "$f" | head -20
done

Length of output: 2498

---

🏁 Script executed:

#!/bin/bash
echo "=== Find LogRowData definition ==="
rg -rn "struct LogRowData" src/

echo -e "\n=== Find log_data module ==="
fd "log_data" src/

echo -e "\n=== Find data_input module ==="
fd "data_input" src/
ls src/

echo -e "\n=== Grep for motor and throttle fields in all rs files ==="
rg -rn "motor\[|rcCommand\[3\]|throttle|\.motor|MotorOutput\|rc_command" src/ | grep -v "constants\|main\.rs\|target/" | head -60

echo -e "\n=== Find where LogRowData fields are defined ==="
rg -rn "pub.*motor\|pub.*throttle\|pub.*setpoint\|pub.*gyro\|pub.*time_sec\|pub.*pid\|pub.*d_term\|pub.*p_term" src/ | grep -v "main\.rs\|target/" | head -60

Length of output: 367

---

🏁 Script executed:

#!/bin/bash
echo "=== Contents of src/data_input/ ==="
ls -la src/data_input/

echo -e "\n=== Contents of src/data_analysis/ ==="
ls -la src/data_analysis/

echo -e "\n=== Find LogRowData definition ==="
grep -rn "struct LogRowData\|LogRowData {" src/

echo -e "\n=== Find throttle and motor fields anywhere in codebase ==="
grep -rn "throttle\|motor\[" src/ | grep -v ".pyc\|Binary" | head -60

echo -e "\n=== Data input files ==="
for f in src/data_input/*; do
  echo "--- $f ---"
  head -80 "$f"
  echo ""
done

Length of output: 15132

---

@nerdCopter, this is a genuinely better long-term direction. I've analyzed the codebase deeply — here's the full picture:

What Data Is Already Available

LogRowData has everything needed:

• setpoint[3] = throttle (0–1000 range, confirmed in log_parser.rs line 82)
• motors: Vec<Option<f64>> = per-motor outputs (dynamic count, typically 4)
• gyro: [Option<f64>; 3] = filtered angular rate (Roll/Pitch/Yaw)
• time_sec: Option<f64> = timestamp

And critically, the hook is already wired in src/main.rs line 1028:

None, // Don't use physics_td_target - empirical targets more accurate

OptimalPAnalysis::analyze() already accepts physics_td_target_ms: Option<(f64, f64)> — it just needs a real computed value instead of None.

---

The Core Physics

The aircraft-specific quantity that eliminates the lookup table is the effective angular acceleration per unit motor command — essentially τ/I without needing to know τ or I separately:

α_eff = dω/dt  (from consecutive gyro samples)
cmd   = motor[0..N].avg() or setpoint[3]  (normalized 0→1)

torque_inertia_ratio = α_eff_peak / cmd_step_size   [rad/s² per unit command]

From this, the aircraft-specific Td target is:

ωn_effective = sqrt(Kp × torque_inertia_ratio)
Td_target_ms = (π / (2 × ωn_effective)) × 1000

No table. No prop size. Pure physics from the log.

---

How Many Logs?

Can 1 log work? Yes — if it contains ≥5 identifiable throttle-punch events (rapid throttle increase ≥20% within ~50ms). Most normal freestyle/racing logs have these organically.

Recommended: 3–5 logs from the same aircraft, same tune. You average the torque_inertia_ratio estimates — the standard deviation tells you how consistent the measurements are (a built-in quality gate).

You do NOT need a dedicated full-sweep log. Throttle punches embedded in normal flight are sufficient. A dedicated 0→100% sweep log would give the cleanest estimate, but it's optional.

---

Proposed Pipeline

flowchart TD
    A["Multiple .csv logs\n(same aircraft)"] --> B["Phase 1:\nThrottle-punch detection\nper log"]
    B --> C["Measure α_eff during\neach punch event\n(d(gyro)/dt vs cmd_step)"]
    C --> D["Aggregate:\nmedian(torque_inertia_ratio)\nper axis across all logs"]
    D --> E{"Sufficient events?\n≥5 punches,\nCV < 30%"}
    E -- Yes --> F["Compute physics Td_target\n= π/(2×√(Kp×τI_ratio))×1000"]
    E -- No --> G["Fallback: prop-size\nlookup table"]
    F --> H["Pass as\nphysics_td_target_ms\nto analyze()"]
    G --> H
    H --> I["Existing OptimalPAnalysis\n(unchanged)"]

Loading

---

Logistics for the Codebase

New module: src/data_analysis/torque_inertia_profiler.rs

/// Detects throttle-punch events and extracts angular acceleration characteristics.
/// Returns Option<(torque_inertia_ratio, tolerance_ratio)> per axis.
pub fn estimate_aircraft_td_target(
    all_log_data: &[LogRowData],
    sample_rate: f64,
    axis: usize,
    current_p: u32,
) -> Option<(f64, f64)> {
    // 1. Find throttle-punch events: setpoint[3] increases > THROTTLE_PUNCH_MIN_DELTA
    //    within THROTTLE_PUNCH_WINDOW_MS
    // 2. For each punch: measure peak d(gyro[axis])/dt in the response window
    // 3. Normalize by throttle command delta
    // 4. Collect all valid torque_inertia_ratio estimates
    // 5. Return (median_td_target_ms, stddev_as_tolerance_ms)
}

Updated call in src/main.rs (replace line 1028):

// Try physics-derived Td target first
let physics_td = if let Some(sr) = sample_rate {
    torque_inertia_profiler::estimate_aircraft_td_target(
        &all_log_data, sr, axis_index, p_gain
    )
} else {
    None
};

// physics_td = None falls back to frame-class target inside analyze()
match OptimalPAnalysis::analyze(
    &td_samples_ms, p_gain, current_d,
    analysis_opts.frame_class, hf_energy_ratio,
    recommended_pd_conservative[axis_index],
    physics_td,  // ← was hardcoded None
) { ... }

---

Multi-log Grouping Strategy

Simplest approach (no code change needed): pass all logs from the same aircraft in a single invocation. The existing multi-file processing already collects all_log_data per file — you'd aggregate the torque_inertia_ratio estimates across files before running step response analysis.

More structured option: a new --calibrate-from <glob> flag that reads additional logs just for the profiling phase, without running full step-response analysis on them.

Filename grouping would require more work (matching HELIO_* vs APEX_*) and is fragile for users with inconsistent naming. I'd recommend against this automatically — let the user pass the right files explicitly.

---

What to Do About --prop-size

Keep it as the fallback with a console message:

  Roll: Using physics-derived Td target: 19.2ms ±3.1ms
         (from 8 throttle-punch events across 2 logs)
  
  Pitch: Insufficient throttle dynamics (3 events < 5 minimum)
         Falling back to prop-size table: 20.0ms ±5.0ms

This makes --prop-size optional when good log data exists.

---

On Propeller Pitch

Agreed that pitch matters outside the ~4.3–5.1" assumed range. The physics-derived approach automatically accounts for pitch — the angular acceleration you measure from the log already reflects whatever prop pitch is installed. This is another advantage over the table.

---

Recommended Action

Rather than updating TD_TARGETS in this PR (Option B from before), I'd suggest:

1. Merge this PR with the table as-is (community testing baseline)
2. New PR for torque_inertia_profiler.rs — this is a meaningful architecture change that deserves its own review
3. Issue #134 still makes sense for collecting 8"+ validation data as a parallel track

Should I open an issue for the physics-based Td derivation, and would you like me to kick off planning the implementation?

---

✏️ Learnings added

Learnt from: nerdCopter
URL: https://github.com/nerdCopter/BlackBox_CSV_Render/pull/132

Timestamp: 2026-05-16T23:21:30.521Z
Learning: In the BlackBox_CSV_Render project, the user nerdCopter wants to replace the empirical TD_TARGETS prop-size lookup table with a physics-derived per-aircraft Td target computed directly from Blackbox log data. The approach: detect throttle-punch events (setpoint[3] rapid increase), measure angular acceleration (d(gyro)/dt) during those events, compute effective torque-to-inertia ratio, and derive Td_target_ms = (π / (2 × sqrt(Kp × τI_ratio))) × 1000. This value is passed as `physics_td_target_ms` to `OptimalPAnalysis::analyze()` (already accepts this parameter, was always passed None). The prop-size table remains as fallback when insufficient throttle events are present. One log with ≥5 throttle punches is minimally viable; 3-5 logs averaged is recommended. No dedicated full-sweep logs required — organic throttle punches from normal flight are sufficient.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

[nerdCopter]

@coderabbitai, if you agree i prefer to purge --prop-size and not have it is a fallback. the empirical 5-in was my 2 different aircraft and in no way should broadly represent all aircraft. Also the file-name grouping should be intelligent enough to accommodate only grouping similar logs (aircraft name grouping)

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== Multi-file processing in main.rs ==="
rg -n "input_files\|process_file\|for.*csv\|filename\|file_name\|stem\|path" src/main.rs | head -60

echo -e "\n=== How files are passed/iterated ==="
sed -n '1,80p' src/main.rs

echo -e "\n=== Betaflight blackbox filename pattern examples - any references ==="
rg -rn "BLACKBOX\|\.bfl\|\.bbb\|file_stem\|file_name\|Path::new\|OsStr" src/ | head -40

echo -e "\n=== process_file signature ==="
rg -n "fn process_file" src/main.rs -A 10

echo -e "\n=== How estimate_optimal_p block references prop_size/frame_class ==="
rg -n "frame_class\|prop_size\|FrameClass\|OptimalPAnalysis\|physics_td" src/main.rs -A 3 | head -80

Length of output: 2742

---

🏁 Script executed:

#!/bin/bash
echo "=== AnalysisOptions struct ==="
rg -n "struct AnalysisOptions\|AnalysisOptions {" src/main.rs -A 15

echo -e "\n=== Argument parsing for prop-size/frame_class ==="
sed -n '/prop.size\|frame.class\|estimate.optimal/I,+20p' src/main.rs | head -80

echo -e "\n=== Optimal P block in process_file ==="
rg -n "estimate_optimal_p\|OptimalPAnalysis\|physics_td\|frame_class\|td_samples" src/main.rs | head -60

echo -e "\n=== How input files are collected and iterated in main() ==="
sed -n '/^fn main/,/^fn process_file/p' src/main.rs | tail -120

echo -e "\n=== File iteration/recursion logic ==="
rg -n "recursive\|walk_dir\|read_dir\|glob\|WalkDir\|input_files\|process_file(" src/main.rs | head -40

Length of output: 8550

---

@nerdCopter — fully agreed on both points. The empirical table never belonged as a fallback; it represents two specific aircraft and will silently mislead anyone whose build doesn't match. Here's the proposed design:

---

1. Purge --prop-size entirely

What gets removed:

• --prop-size CLI flag
• FrameClass as a Td-target source
• The require --prop-size when --estimate-optimal-p enforcement
• frame_class: FrameClass from AnalysisOptions
• TD_TARGETS as the primary lookup (can keep TdTargetSpec struct for future use; or remove entirely)
• The FiveInch default in AnalysisOptions construction

What replaces it:

--estimate-optimal-p now requires sufficient log data to derive Td.
If <5 throttle-punch events found across all logs in the group → warn + skip analysis.
No silent fallback to a table.

Console behavior when physics derivation fails:

--- Optimal P Estimation ---
Roll: SKIPPED — insufficient throttle dynamics detected (2 events, need ≥5).
      Provide logs with more throttle variation or fly a deliberate punch sequence.

FrameClass can be kept as a display label only (e.g. in legend text: "estimated from log dynamics") without ever being used for Td targets.

---

2. Intelligent filename-based aircraft grouping

Betaflight filenames follow a consistent pattern:

EMUF_BLACKBOX_LOG_HELIO_V1_20260103_160240.02.csv
└──────────────────────────┘└────────┘└──────┘└──┘
       craft key (group by)    date     time   seq

Grouping key extraction — strip the date/time/sequence suffix using the pattern _\d{8}_\d{6} and everything after it:

fn extract_aircraft_key(path: &Path) -> String {
    let stem = path.file_stem()
        .and_then(|s| s.to_str())
        .unwrap_or("");
    
    // Strip sequence number (.02 → part of stem in some decoders, handled above)
    // Strip _YYYYMMDD_HHMMSS suffix (8-digit date, 6-digit time)
    let re = regex::Regex::new(r"_\d{8}_\d{6}.*$").unwrap();
    let key = re.replace(stem, "").to_string();
    
    // Fallback: if no date pattern found, use full stem (unique aircraft)
    if key == stem { stem.to_string() } else { key }
}

This groups:

EMUF_BLACKBOX_LOG_HELIO_V1_20260103_160240.02.csv  →  EMUF_BLACKBOX_LOG_HELIO_V1
EMUF_BLACKBOX_LOG_HELIO_V1_20260103_161530.01.csv  →  EMUF_BLACKBOX_LOG_HELIO_V1
EMUF_BLACKBOX_LOG_APEX_20260105_090000.01.csv      →  EMUF_BLACKBOX_LOG_APEX

New processing architecture:

flowchart TD
    A["All input .csv files"] --> B["extract_aircraft_key()\nper file"]
    B --> C["HashMap: key → Vec of file paths"]
    C --> D["For each aircraft group"]
    D --> E["Phase 1: torque_inertia_profiler\nacross ALL logs in group\n→ physics Td target + tolerance"]
    E --> F{"≥5 throttle\npunch events?"}
    F -- Yes --> G["physics_td_target_ms = Some(td, tol)"]
    F -- No --> H["Warn + skip optimal-P\nfor this group"]
    G --> I["Phase 2: process_file() per log\nwith shared physics Td target"]
    H --> I

Loading

Architectural change to main.rs:

// Before: flat iteration
for input_file_str in &input_files {
    process_file(input_file_str, ..., analysis_opts)?;
}

// After: grouped iteration
let groups = group_files_by_aircraft(&input_files);
for (craft_key, files) in &groups {
    let physics_td = if analysis_opts.estimate_optimal_p {
        torque_inertia_profiler::estimate_from_logs(files, sample_rate)
    } else {
        None
    };
    for file in files {
        process_file(file, ..., analysis_opts, physics_td)?;
    }
}

---

Edge Cases

Scenario	Behavior
Mixed aircraft in one invocation	Each group gets its own physics Td target
Non-Betaflight filename (no date pattern)	Full stem = group key → treated as unique aircraft
Single log, ≥5 punches	Works fine
Single log, <5 punches	Warn + skip; suggest providing more logs
--recursive with multiple aircraft dirs	Each aircraft groups naturally by filename key

---

Summary of Changes Needed

• Remove --prop-size, FrameClass Td-target logic, TD_TARGETS (or archive to a comment)
• Add extract_aircraft_key() filename parser
• Add group_files_by_aircraft() in main.rs
• Implement torque_inertia_profiler::estimate_from_logs() (new module)
• Change process_file() signature to accept physics_td_target: Option<(f64, f64)> directly
• No-fallback failure path with clear console warnings

Shall I open a new issue (or plan the implementation here) with these specifics?

---

✏️ Learnings added

Learnt from: nerdCopter
URL: https://github.com/nerdCopter/BlackBox_CSV_Render/pull/132

Timestamp: 2026-05-17T22:46:46.857Z
Learning: In the BlackBox_CSV_Render project, nerdCopter wants to purge the --prop-size flag and the empirical TD_TARGETS lookup table entirely from the optimal-P estimation feature. There is to be NO fallback to any empirical table. If physics-based Td cannot be derived from log data (insufficient throttle-punch events, minimum 5), the analysis should be skipped with a clear console warning. FrameClass may remain as a display label only, not as a Td-target source.

---

Learnt from: nerdCopter
URL: https://github.com/nerdCopter/BlackBox_CSV_Render/pull/132

Timestamp: 2026-05-17T22:46:46.857Z
Learning: In the BlackBox_CSV_Render project, nerdCopter wants intelligent aircraft-based grouping of input log files for the physics-derived Td estimation feature. Betaflight blackbox filenames follow the pattern {PREFIX}_BLACKBOX_LOG_{CRAFT_NAME}_{YYYYMMDD}_{HHMMSS}.{SEQ}.csv. The grouping key is extracted by stripping the _YYYYMMDD_HHMMSS and everything after it from the filename stem. Files with the same key are treated as the same aircraft and their log data is aggregated for the torque-inertia profiler phase before individual file processing.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

@nerdCopter — it looks like this comment was already addressed in the thread above, but given the conversation has progressed further, here's a consolidated summary of where things stand:

Agreed Direction (from the full thread)

Your core questions, answered:

1. Full-sweep throttle logs not required — organic throttle punches from normal freestyle/racing flights are sufficient (rapid setpoint[3] increase ≥20% within ~50ms). A dedicated sweep is cleaner but not needed.

2. Minimum effective log count: 1, if it contains ≥5 identifiable throttle-punch events. 3–5 logs averaged is the recommended target; the std-dev of the per-log estimates becomes a built-in quality gate.

3. Processing order: Phase 1 (torque-inertia profiling across all logs for the group) → derive physics Td_target_ms → Phase 2 (per-log step response + optimal P) using that shared target. Not per-log separate then averaged — aggregate the punch-event samples first.

4. --prop-size purged entirely — no fallback to the empirical table. If <5 throttle-punch events are found across all logs in the group, skip optimal-P with a clear console warning. TD_TARGETS / FrameClass as Td-target source removed.

5. Filename grouping — strip _YYYYMMDD_HHMMSS suffix from the Betaflight filename stem to form the aircraft key:

   EMUF_BLACKBOX_LOG_HELIO_V1_20260103_160240.02.csv → EMUF_BLACKBOX_LOG_HELIO_V1
   

   Files with the same key are aggregated for the profiling phase before per-file processing. Non-matching filenames → unique key (no grouping).

What the Hook Already Looks Like

main.rs line 1028 already passes None to OptimalPAnalysis::analyze() for physics_td_target_ms. The new torque_inertia_profiler module just needs to produce a Some((td_ms, tolerance_ms)) to drop in there — minimal invasive change to existing analysis code.

Proposed Next Steps

The implementation is a meaningful architecture change from the current PR's design — I'd recommend:

• Merge this PR as-is (feature is DO NOT MERGE labeled, community baseline)
• New branch/PR for torque_inertia_profiler.rs + --prop-size removal + filename grouping

Would you like me to open a GitHub issue capturing all the above specs (physics Td derivation, no-fallback requirement, filename grouping algorithm) so it can be tracked independently of this PR?

[nerdCopter]

as a result I(AI) opened #143 and #141 (141 needs re-basing after 143 potentially merges).

side-note: i'm overusing Claude (usage limits), so i started using Gemini which seems to be okay with math/physics, but is too eager to agree with human-engineer, has horrible patching-logic, and also fails to follow directions often. Crossing my fingers 🤞 ... but also was forced to reproduce #143 via Claude-Code from scratch/documentation.

@coderabbitai, can you investigate #143 on the optimal P/D tuning aspects? then investigate #141 as the solution to our former conversation about "feature/torque-inertia-profiler"

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

Sure, @nerdCopter — let me pull up both PRs and dig into the details.

---

🏁 Script executed:

#!/bin/bash
echo "=== PR `#143` details ==="
gh pr view 143 --repo nerdCopter/BlackBox_CSV_Render --json title,body,headRefName,baseRefName,labels,state,author,createdAt,files,commits | jq '{title,headRefName,baseRefName,labels,state,author,createdAt}'

echo -e "\n=== PR `#143` description ==="
gh pr view 143 --repo nerdCopter/BlackBox_CSV_Render --json body | jq -r '.body' | head -200

echo -e "\n=== PR `#143` changed files ==="
gh pr view 143 --repo nerdCopter/BlackBox_CSV_Render --json files | jq -r '.files[].path'

echo -e "\n=== PR `#143` comments (first 10) ==="
gh pr view 143 --repo nerdCopter/BlackBox_CSV_Render --json comments | jq -r '.comments[] | "\(.author.login): \(.body)"' | head -100

Length of output: 11260

---

Got #143. Now pulling #141:

---

🏁 Script executed:

#!/bin/bash
echo "=== PR `#141` details ==="
gh pr view 141 --repo nerdCopter/BlackBox_CSV_Render --json title,body,headRefName,baseRefName,labels,state,author,createdAt | jq '{title,headRefName,baseRefName,labels,state,author,createdAt}'

echo -e "\n=== PR `#141` description ==="
gh pr view 141 --repo nerdCopter/BlackBox_CSV_Render --json body | jq -r '.body' | head -300

echo -e "\n=== PR `#141` changed files ==="
gh pr view 141 --repo nerdCopter/BlackBox_CSV_Render --json files | jq -r '.files[].path'

echo -e "\n=== PR `#141` comments ==="
gh pr view 141 --repo nerdCopter/BlackBox_CSV_Render --json comments | jq -r '.comments[] | "\(.author.login): \(.body)"' | head -150

echo -e "\n=== PR `#141` review comments ==="
gh api repos/nerdCopter/BlackBox_CSV_Render/pulls/141/reviews | jq -r '.[] | "\(.user.login): \(.body)"' | head -60

Length of output: 5030

---

Good, now let me pull the actual code from both branches for deeper analysis:

---

🏁 Script executed:

#!/bin/bash
echo "=== PR `#143` branch: constants.rs changes ==="
gh api repos/nerdCopter/BlackBox_CSV_Render/contents/src/constants.rs?ref=step-response-thresholds \
  | jq -r '.content' | base64 -d | grep -n "PEAK_\|PD_RATIO\|P_HEADROOM\|OVERSHOOT\|UNDERSHOOT\|AGGRESSIVE\|MODERATE\|CONSERVATIVE\|OPTIMAL\|ACCEPTABLE" | head -60

echo -e "\n=== PR `#143` diff for constants.rs ==="
gh api repos/nerdCopter/BlackBox_CSV_Render/pulls/143/files \
  | jq -r '.[] | select(.filename == "src/constants.rs") | .patch' | head -120

echo -e "\n=== PR `#141`: new torque_inertia_profiler.rs ==="
gh api repos/nerdCopter/BlackBox_CSV_Render/contents/src/data_analysis/torque_inertia_profiler.rs?ref=feature/torque-inertia-profiler \
  | jq -r '.content' | base64 -d | head -200

echo -e "\n=== PR `#141` diff for optimal_p_estimation.rs (key parts) ==="
gh api repos/nerdCopter/BlackBox_CSV_Render/pulls/141/files \
  | jq -r '.[] | select(.filename == "src/data_analysis/optimal_p_estimation.rs") | .patch' | head -120

Length of output: 17995

---

🏁 Script executed:

#!/bin/bash
echo "=== PR `#141` constants - TORQUE_PROFILER_* values ==="
gh api repos/nerdCopter/BlackBox_CSV_Render/contents/src/constants.rs?ref=feature/torque-inertia-profiler \
  | jq -r '.content' | base64 -d | grep -n "TORQUE_PROFILER\|THROTTLE_PUNCH\|ACHIEVABILITY\|TD_CALC\|P_SCALE\|TD_TARGETS\|TdTargetSpec\|OPTIMAL_P" | head -60

echo -e "\n=== PR `#141` constants - full torque profiler section ==="
gh api repos/nerdCopter/BlackBox_CSV_Render/contents/src/constants.rs?ref=feature/torque-inertia-profiler \
  | jq -r '.content' | base64 -d | sed -n '/TORQUE_PROFILER\|THROTTLE_PUNCH/,/^$/p' | head -80

echo -e "\n=== PR `#141` main.rs - aircraft grouping and profiling invocation ==="
gh api repos/nerdCopter/BlackBox_CSV_Render/contents/src/main.rs?ref=feature/torque-inertia-profiler \
  | jq -r '.content' | base64 -d | grep -n "group\|aircraft\|profil\|torque\|physics_td\|punch\|AircraftProfile\|extract_aircraft_key\|torque_inertia" | head -60

echo -e "\n=== PR `#141` main.rs - physics_td and estimate_optimal_p block ==="
gh api repos/nerdCopter/BlackBox_CSV_Render/contents/src/main.rs?ref=feature/torque-inertia-profiler \
  | jq -r '.content' | base64 -d | grep -n "physics_td\|estimate_optimal_p\|aircraft_profile\|AircraftProfile\|aircraft_key\|skip_reason" | head -50

echo -e "\n=== PR `#141` diff for main.rs - grouping section ==="
gh api repos/nerdCopter/BlackBox_CSV_Render/pulls/141/files \
  | jq -r '.[] | select(.filename == "src/main.rs") | .patch' | head -200

Length of output: 15473

---

@nerdCopter — great progress on both PRs. Here's my full analysis:

---

PR #143 — Step Response Tuning Zones

Philosophy ✅ Correct

The new 6-zone model is aligned with how real-world multirotor tuning actually works:

Zone	Range	Recommendation
Undershoot	< 1.00	Proportional D decrease → target 1.05
Near Optimal	1.00–1.02	None (optional D−1 hint)
Sweet Spot	1.02–1.08	None
Acceptable	1.08–1.12	Conservative only (+2% D)
Overshoot	1.12–1.20	Conservative + Moderate
Significant	> 1.20	All three stacked

The old PEAK_OPTIMAL_MIN = 0.95 threshold was wrong — calling zero-overshoot "optimal" contradicted the entire rationale from Brian White's insight. Good removal.

Specific observations

1. PEAK_OPTIMAL_TARGET is computed, not hardcoded ((1.02 + 1.08) / 2.0 = 1.05) — correct approach; if the sweet-spot bounds shift in the future, the target follows automatically.

2. Proportional undershoot formula P:D × (PEAK_OPTIMAL_TARGET / peak.max(PEAK_VALUE_MIN_CLAMP)) — elegant and correct. PEAK_VALUE_MIN_CLAMP = 0.1 correctly guards against divide-by-zero.

3. Near-optimal D−1 hint — small concern: when current_d is already at some pilot-set minimum (e.g., D=5), the hint produces D=4, which could visually look like an active recommendation rather than an optional fine-tune. The word "hint" vs "Recommendation" in the label is important — verify the output text makes the optionality clear.

4. +2% D via PEAK_ACCEPTABLE_MULTIPLIER = 0.98 — for small P values (e.g., P=30, P:D=1.5 → D=20), the computed D change would be 20 × 1/0.98 ≈ 20.4 → D=20. That rounds to no change. Worth adding a minimum integer step guard (e.g., max(recommended_d, current_d + 1)) when the zone isn't optimal.

5. MIN_REASONABLE_PD_RATIO lowered from 0.5 → 0.3 — correct for large aircraft. Worth noting in the PR description that this accommodates 10"+ builds where D legitimately exceeds P.

6. PD_RATIO_AGGRESSIVE_MULTIPLIER = 0.65 — only used for peak > 1.20. At this multiplier, D increases ~54% which is significant. Ensure this only triggers with the full stacked recommendation text so pilots know the severity.

---

PR #141 — Torque-Inertia Profiler

Architecture ✅ Correct Direction

The two-phase approach (Phase 1: profiling → Phase 2: per-file analysis) is the right design. The hook in main.rs is clean:

flowchart LR
    A["input files"] --> B["group_files_by_aircraft()"]
    B --> C["profile_aircraft_group()\nPhase 1: punch detection"]
    C --> D["AircraftProfile per group"]
    D --> E["process_file() per log\nPhase 2: step analysis\nwith physics Td target"]

Loading

Key observations

✅ Strengths

• TORQUE_PROFILER_TD_CALC_K = 1570.796... = π/2 × 1000 — correctly derived from Td = π/(2ωn) × 1000ms. The math checks out.
• AxisProfile::from_ratios uses median (robust to outliers from noisy punches) + half-IQR spread as tolerance — better than mean/std for this use case.
• Graceful degradation: profiling parse failures silently skip the file, don't abort.
• No fallback to empirical table — exactly as discussed. Skip + clear console message.

⚠️ Issues to Address

1. BTFL filename grouping failure (important)

extract_aircraft_key scans for _YYYYMMDD_HHMMSS left-to-right. For Betaflight-style filenames like:

BTFL_BLACKBOX_LOG_20250517_130413_STELLARH7DEV.02.csv

The date pattern _20250517_130413 occurs at byte position 15, giving key BTFL_BLACKBOX_LOG — meaning all Betaflight-named files in a directory would be grouped as one aircraft. The code comment acknowledges this. This is a serious problem with --recursive on mixed-aircraft folders.

One approach: check if the extracted key is a generic prefix with no craft-identifying content (e.g., BTFL_BLACKBOX_LOG without anything meaningful after LOG_). If so, fall back to filename-unique grouping and print a warning:

[torque-profile] Warning: Could not identify craft name in BTFL_BLACKBOX_LOG_20250517_*.csv
                 Files will be treated as individual aircraft.
                 Rename files with a craft identifier before the date for grouping.

Alternatively, detect whether the suffix after stripping the date contains non-trivial content and use that as the key.

2. Double file parsing (performance)

Each file is parsed by parse_log_file twice — once in profile_aircraft_group and once in process_file. For a typical 10-minute flight log at 1kHz this is ~600K rows parsed twice. For a 20-file --recursive run this matters. Consider either:

• Caching LogRowData per file during Phase 1 and passing it to Phase 2 (uses more RAM), or
• Documenting the double-parse as a known cost in a TODO comment.

3. TORQUE_PROFILER_ACHIEVABILITY_FACTOR = 2.50 is empirically derived from HELIO only

The PR says this was validated on HELIO (~16ms). A factor of 2.5× means the raw theoretical Td must be ~6.4ms before scaling. This factor will differ for:

• A heavy 7" long-range at lower power-to-inertia ratio
• A tiny 3" toothpick with very high ratio

It's better than a static prop-size table, but it's still one empirical constant. Document clearly in constants.rs that this was calibrated against a 5" 6S freestyle build and may need user adjustment as community data expands.

4. TORQUE_PROFILER_SETTLE_SAMPLES = 3 may be too short

At 1kHz, 3 samples = 3ms settle time before measuring angular acceleration. For slower-responding large aircraft (8"+), the ESC and motor lag alone can be 5–10ms. You might miss the actual acceleration peak and measure noise instead. Consider increasing to 5–8 samples, or making it proportional to the detected sample rate.

5. Yaw axis profiled but never used

extract_punch_ratios returns ratios for all 3 axes, and AircraftProfile::from_axis_ratios stores all 3, but the optimal-P analysis only iterates ROLL_PITCH_AXIS_COUNT = 2. Yaw profiling work is silently discarded. Minor cleanup: either skip Yaw in extract_punch_ratios or add a note in the profiler summary.

---

Merge Order / Rebase Note

Since #141 depends on #143's constants (PEAK_* thresholds), the order is:

1. Merge #143 first
2. Rebase #141 onto the updated master
3. #141 carries the AnalysisOptions struct and estimate_optimal_p flag without frame_class — verify the rebase removes any remaining FrameClass imports cleanly

---

Summary

	#143 Tuning Zones	#141 Torque Profiler
Core approach	✅ Correct	✅ Correct direction
Critical issues	None	BTFL filename grouping
Important concerns	D-1 hint UX, +2% rounding	Double parse, achievability factor docs
Minor	None	Yaw axis profiling unused
Test coverage	212 CSVs ✅	HELIO/FOXEER/APEX ✅

Both PRs are structurally solid. The BTFL grouping issue in #141 is the one item worth addressing before merging — it would silently produce incorrect Td targets for mixed-aircraft --recursive runs. Everything else is either polish or documentation.

🐇 (•ω•) [torque profiling intensifies]

---

✏️ Learnings added

Learnt from: nerdCopter
URL: https://github.com/nerdCopter/BlackBox_CSV_Render/pull/132

Timestamp: 2026-05-20T18:24:29.670Z
Learning: In the BlackBox_CSV_Render project, PR `#141` (feature/torque-inertia-profiler) introduces a two-phase aircraft-based file grouping architecture in main.rs: Phase 1 calls profile_aircraft_group() across all files with the same aircraft key, Phase 2 calls process_file() per file with the resulting AircraftProfile. The aircraft key is extracted by scanning for _YYYYMMDD_HHMMSS in the filename stem. A known limitation: Betaflight-style filenames (BTFL_BLACKBOX_LOG_YYYYMMDD_HHMMSS_CRAFTNAME.csv) have the date before the craft name, causing all BTFL files to share key "BTFL_BLACKBOX_LOG". EMUF-style filenames (EMUF_BLACKBOX_LOG_CRAFTNAME_YYYYMMDD_HHMMSS.csv) group correctly.

---

Learnt from: nerdCopter
URL: https://github.com/nerdCopter/BlackBox_CSV_Render/pull/132

Timestamp: 2026-05-20T18:24:29.670Z
Learning: In the BlackBox_CSV_Render project, PR `#143` (step-response-thresholds) reworks the P:D tuning zone thresholds to 6 zones: <1.00 undershoot (proportional D decrease targeting PEAK_OPTIMAL_TARGET=1.05), 1.00-1.02 near-optimal (none + optional D-1 hint), 1.02-1.08 optimal sweet spot (none), 1.08-1.12 acceptable (conservative +2% D via PEAK_ACCEPTABLE_MULTIPLIER=0.98), 1.12-1.20 overshoot (conservative + moderate), >1.20 significant (all three). Also adds PD_RATIO_AGGRESSIVE_MULTIPLIER=0.65 and lowers MIN_REASONABLE_PD_RATIO from 0.5 to 0.3 for large aircraft where D > P.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

[nerdCopter]

@coderabbitai — feedback on your PR #143 observations, items 3 and 4:

Item 3 — D−1 hint label optionality: Already handled. The current output reads Recommendation (conservative): D≈{n} (P={p}) [optional D−1] — the [optional D−1] suffix makes the optionality explicit. Non-issue.

Item 4 — +2% rounding to no change: Confirmed real. For small P values (e.g. P=30, D=20, P:D=1.5), recommended_d = round(30 / 1.47) = 20 — identical to current D, so the Acceptable zone would display Recommendation (conservative): D≈20 when D is already 20.

Two approaches were considered:

• Min +1 guard: clamp max(rec_d, current_d + 1) for the Acceptable zone — guarantees a visible actionable nudge.
• Suppress when identical: skip storing the recommendation if computed D equals current D.

Decision: leaving as a known limitation for now. The edge case is narrow (only bites when P is small relative to D), and adding the guard introduces zone-specific logic in the computation path that adds complexity disproportionate to the benefit. If it surfaces in real logs it will be revisited.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

@nerdCopter — acknowledged on both:

Item 3: Good design choice — [optional D−1] inline in the label is clean and unambiguous. Non-issue confirmed.

Item 4: Understood. The decision is sound — the edge case only bites on low-P builds where the Acceptable zone (+2%) rounds to exactly zero delta, and adding zone-specific integer clamping buys little against the complexity cost. Recording this as a documented known limitation so it doesn't get re-raised in future reviews.

---

✏️ Learnings added

Learnt from: nerdCopter
URL: https://github.com/nerdCopter/BlackBox_CSV_Render/pull/132

Timestamp: 2026-05-20T18:42:08.038Z
Learning: In the BlackBox_CSV_Render project, PR `#143` (step-response-thresholds): the Acceptable zone (+2% D via PEAK_ACCEPTABLE_MULTIPLIER=0.98) has a known limitation where small P values (e.g. P=30, D=20) round to no D change (recommended_d == current_d). This was deliberately left as a known limitation; both a min-+1 guard and suppress-when-identical approaches were considered and rejected as disproportionate complexity for a narrow edge case. Will be revisited if surfaced in real logs.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

[nerdCopter]

mistakenly merged, then reverted, then force-pushed old master.

[nerdCopter]

@coderabbitai — Following up on the CodeRabbit review at #132 (comment 4501386876) regarding PR #141. #141 has been closed and superseded by #145 (feature/tip-squash-rebase), a clean squash-rebase onto master.

Changes made in response to that feedback in #145:

[Important] BTFL filename grouping fix — extract_aircraft_key() now appends the craft name from the post-date suffix when the prefix ends with BLACKBOX_LOG. BTFL_BLACKBOX_LOG_YYYYMMDD_HHMMSS_CRAFTNAME.NN.csv now correctly keys to BTFL_BLACKBOX_LOG_CRAFTNAME instead of collapsing all files to BTFL_BLACKBOX_LOG.

TORQUE_PROFILER_SETTLE_SAMPLES — Increased from 3 to 5 samples, covering ~5 ms ESC/motor lag at 1 kHz (addresses the concern that 3 ms may be too short for larger/slower aircraft).

TORQUE_PROFILER_ACHIEVABILITY_FACTOR — Doc comment updated to note empirical calibration on a 5" 6S freestyle build (HELIO H7) and that adjustment may be needed for other aircraft classes.

Yaw axis — Added comment to extract_punch_ratios() clarifying that axis 2 (Yaw) is collected for diagnostic completeness but is excluded from optimal-P analysis.

Double-parse TODO — Noted as a performance concern; not fixed in this PR (correctness is unaffected).

Additionally, two verbiage/plot issues found during rebase review were fixed:

• "Conservative recommendation:" / "Moderate recommendation:" in plot_step_response.rs normalised to "Recommendation (conservative):" / "Recommendation (moderate):" (matching main.rs console output)
• Duplicate "Recommendation: None (Optimal)" legend entry removed from PNG when axis is in optimal zone

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== PR `#145` details ==="
gh pr view 145 --repo nerdCopter/BlackBox_CSV_Render --json title,headRefName,baseRefName,state,author | jq .

echo -e "\n=== PR `#145` changed files ==="
gh pr view 145 --repo nerdCopter/BlackBox_CSV_Render --json files | jq -r '.files[].path'

echo -e "\n=== extract_aircraft_key in `#145` ==="
gh api repos/nerdCopter/BlackBox_CSV_Render/contents/src/main.rs?ref=feature/tip-squash-rebase \
  | jq -r '.content' | base64 -d | grep -n "extract_aircraft_key\|BLACKBOX_LOG\|BTFL\|craft_name\|post_date\|after_date\|suffix" | head -40

echo -e "\n=== full extract_aircraft_key function ==="
gh api repos/nerdCopter/BlackBox_CSV_Render/contents/src/main.rs?ref=feature/tip-squash-rebase \
  | jq -r '.content' | base64 -d | sed -n '/fn extract_aircraft_key/,/^}/p' | head -60

echo -e "\n=== TORQUE_PROFILER constants in `#145` ==="
gh api repos/nerdCopter/BlackBox_CSV_Render/contents/src/constants.rs?ref=feature/tip-squash-rebase \
  | jq -r '.content' | base64 -d | grep -n "TORQUE_PROFILER\|SETTLE_SAMPLES\|ACHIEVABILITY" -A 3 | head -60

echo -e "\n=== Yaw comment in extract_punch_ratios ==="
gh api repos/nerdCopter/BlackBox_CSV_Render/contents/src/data_analysis/torque_inertia_profiler.rs?ref=feature/tip-squash-rebase \
  | jq -r '.content' | base64 -d | grep -n "yaw\|Yaw\|axis 2\|diagnostic\|exclude\|optimal-P\|optimal_p" | head -20

echo -e "\n=== plot label normalization in plot_step_response.rs ==="
gh api repos/nerdCopter/BlackBox_CSV_Render/contents/src/plot_functions/plot_step_response.rs?ref=feature/tip-squash-rebase \
  | jq -r '.content' | base64 -d | grep -n "Recommendation (conservative)\|Recommendation (moderate)\|Conservative recommendation\|Moderate recommendation\|None (Optimal)\|Optimal)" | head -30

Length of output: 6055

---

@nerdCopter — verified all changes against the #141 feedback. All items confirmed in feature/tip-squash-rebase:

Feedback Item	Status in #145
BTFL grouping fix	✅ extract_aircraft_key() detects BLACKBOX_LOG prefix, appends craft name from post-date suffix → BTFL_BLACKBOX_LOG_STELLARH7DEV
TORQUE_PROFILER_SETTLE_SAMPLES	✅ Increased 3 → 5; doc comment explicitly notes ~5ms ESC/motor lag at 1 kHz
TORQUE_PROFILER_ACHIEVABILITY_FACTOR docs	✅ Doc comment updated (empirical calibration on HELIO H7, 5" 6S freestyle)
Yaw axis comment	✅ Line 196–197: "Axis 2 (Yaw) is collected here for diagnostic completeness but is not used in optimal-P analysis"
Plot label normalization	✅ All instances now read "Recommendation (conservative):" / "Recommendation (moderate):"
Duplicate optimal legend entry	✅ "None (Optimal)" pattern absent from plot_step_response.rs
Double-parse TODO	✅ Documented as known limitation; no code change needed now

The BTFL grouping logic is particularly clean — the dot-stripping for trailing numeric log sequences (.02, .19) is a nice touch that prevents the sequence number from bleeding into the craft key.

Shall I trigger a full review of #145?

🐇 (・ω・)ノ

---

✏️ Learnings added

Learnt from: nerdCopter
URL: https://github.com/nerdCopter/BlackBox_CSV_Render/pull/132

Timestamp: 2026-05-20T20:21:51.303Z
Learning: In the BlackBox_CSV_Render project, PR `#145` (feature/tip-squash-rebase) supersedes PR `#141` and addresses all `#141` review feedback: BTFL filename grouping fixed in extract_aircraft_key() by detecting "BLACKBOX_LOG" suffix prefix and appending craft name from post-date portion; TORQUE_PROFILER_SETTLE_SAMPLES increased from 3 to 5; TORQUE_PROFILER_ACHIEVABILITY_FACTOR doc updated noting empirical calibration on HELIO H7 5" 6S freestyle; Yaw axis comment added to extract_punch_ratios(); plot labels normalized to "Recommendation (conservative):" / "Recommendation (moderate):"; duplicate "Recommendation: None (Optimal)" legend entry removed; double-parse performance issue documented as TODO.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.