BUG: fins failing to accept both sweep and angle

[aasitvora99]

Saw an error with fin sweep declaration even after sending null value for them. Temp fixing this until lib works on a proper fix.

Stacktrace:

2025-10-05 01:24:46,930 - ERROR - get_flight_simulation: Unexpected error Cannot use sweep_length and sweep_angle together
Traceback (most recent call last):
  File "/Infinity-API/src/controllers/interface.py", line 16, in wrapper
    return await method(self, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Infinity-API/src/controllers/flight.py", line 107, in get_flight_simulation
    flight_service = FlightService.from_flight_model(flight.flight)
  File "/Infinity-API/src/services/flight.py", line 34, in from_flight_model
    rocketpy_rocket = RocketService.from_rocket_model(flight.rocket).rocket
                      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^
  File "/Infinity-API/src/services/rocket.py", line 69, in from_rocket_model
    rocketpy_finset_list = cls.get_rocketpy_finset_list_from_fins_list(
        rocket.fins
    )
  File "/Infinity-API/src/services/rocket.py", line 150, in get_rocketpy_finset_list_from_fins_list
    cls.get_rocketpy_finset(fins, fins.fins_kind) for fins in fins_list
    ~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^
  File "/Infinity-API/src/services/rocket.py", line 164, in get_rocketpy_finset
    rocketpy_finset = RocketPyTrapezoidalFins(
        n=fins.n,
    ...<3 lines>...
        **fins.get_additional_parameters(),
    )
  File "/RocketPy/rocketpy/rocket/aero_surface/fins/trapezoidal_fins.py", line 174, in __init__
    raise ValueError("Cannot use sweep_length and sweep_angle together")
ValueError: Cannot use sweep_length and sweep_angle together

Summary by CodeRabbit

• Breaking Changes

  • Fins may omit sweep_length when both sweep_angle and sweep_length are provided; exported/serialized outputs and additional parameters may no longer include sweep_length in that case.

• Changes

  • Additional parameters for Fins now consistently exclude core fields, producing cleaner/consistent serialized outputs.
  • JSON encoding interface had a parameter rename but preserves public behavior.

• Chores

  • Development Makefile updated to prefer virtualenv tool binaries, add build/cleanup targets, and standardize tooling commands.

• Refactor

  • Internal attribute-collection logic reorganized for simpler, more maintainable behavior with no change to returned results.

[coderabbitai]

Walkthrough

Added a class-level _base_keys to Fins and refactored its get_additional_parameters; introduced new helper functions to simplify collect_attributes population logic in src/utils.py; and updated the Makefile to prefer virtualenv tool binaries, export PYLINTHOME, add build/clean targets, and use tool variables.

Changes

Cohort / File(s)	Summary of Changes
Aerosurface model update src/models/sub/aerosurfaces.py	Added public class attribute _base_keys: Set[str] to Fins; refactored get_additional_parameters to compute additional parameters by excluding _base_keys, filter out None values, and drop sweep_length when both sweep_angle and sweep_length exist.
Utilities refactor src/utils.py	Removed InfinityEncoder.__init__; renamed InfinityEncoder.default(self, obj) → default(self, o) with in-function reassignment; made Flight post-process a method call; replaced monolithic collect_attributes logic with helper functions _populate_simulation_attributes, _annotation_keys, _resolve_attribute_path, _resolve_attribute_target, and _copy_missing_attributes to centralize attribute population.
Makefile improvements Makefile	Added VENV_BIN and PYLINTHOME defaults and export; resolved tool commands to prefer venv binaries with system fallbacks for black, ruff, flake8, pylint, pytest, uvicorn; updated targets to use these variables; added mkdir -p for pylint runtime dir; extended clean with Docker cleanup and added build target; declared new .PHONY targets.

Sequence Diagram(s)

sequenceDiagram
    participant Caller
    participant FinsModel as Fins
    rect rgb(240,248,255)
    Caller->>FinsModel: call get_additional_parameters()
    note right of FinsModel: 1) model_dict = self.dict()
    end
    rect rgb(245,245,245)
    FinsModel->>FinsModel: exclude keys in _base_keys from model_dict
    FinsModel->>FinsModel: remove entries with value == None
    FinsModel->>FinsModel: if sweep_angle and sweep_length present → remove sweep_length
    end
    FinsModel-->>Caller: return additional_parameters (dict)

Loading

sequenceDiagram
    participant Caller
    participant Utils as collect_attributes
    participant Resolver as _resolve_attribute_path
    participant Populator as _populate_simulation_attributes
    rect rgb(240,255,240)
    Caller->>Utils: call collect_attributes(obj)
    note right of Utils: prepare attributes dict & dispatcher table
    end
    rect rgb(255,250,240)
    Utils->>Populator: _populate_simulation_attributes(obj, attr_class, attributes)
    Populator->>Resolver: resolve source paths via _resolve_attribute_path
    Populator->>Utils: _annotation_keys to get keys
    Populator->>Utils: _resolve_attribute_target to ensure nested targets
    Populator->>Utils: _copy_missing_attributes(source, target, keys)
    end
    Utils-->>Caller: return encoded attributes

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

• Gui-FernandesBR
• phmbressan

Poem

A rabbit hops through lines of code,
Keys tucked in a tidy node.
Helpers light the attribute trail,
Makefile tools find venv to hail.
Clean and neat—small changes that glow. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title explicitly identifies the core issue being resolved—allowing fins to accept both sweep length and sweep angle—which matches the changes in the Fins class and the bug described in the PR objectives.

✨ 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 bug/fin-sweep

---

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

[Gui-FernandesBR]

The behavior on RocketPy lib seems to be fine. My guess is that rocketpy (lib) calculates and sets the sweep angle and length after initialization, the infinity tries to recreate the object using these values

[Gui-FernandesBR]

@aasitvora99 I think this line of code is more problematic than the one you have shared

[Gui-FernandesBR]

LGTM

[coderabbitai]

Actionable comments posted: 0

Caution

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

⚠️ Outside diff range comments (1)

Makefile (1)

76-76: Declare composite targets as PHONY.

The .PHONY declaration includes individual targets but omits the composite targets lint and format defined at lines 42-43. This can cause issues if files named lint or format exist in the directory.

Apply this diff:

-.PHONY: black flake8 pylint test dev clean build ruff format
+.PHONY: black flake8 pylint test dev clean build ruff format lint

🧹 Nitpick comments (1)

src/utils.py (1)

57-58: Simplify parameter naming.

The parameter rename from obj to o followed by immediate reassignment obj = o is unnecessary. The original parameter name obj is more descriptive and aligns with the rest of the method body.

Apply this diff:

-    def default(self, o):
-        obj = o
+    def default(self, obj):

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 60a7aea and 57e0159.

📒 Files selected for processing (3)

• Makefile (1 hunks)
• src/models/sub/aerosurfaces.py (1 hunks)
• src/utils.py (3 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• src/models/sub/aerosurfaces.py

🧰 Additional context used

🧬 Code graph analysis (1)

src/utils.py (4)

src/views/flight.py (1)

• FlightSimulation (9-140)

src/views/rocket.py (1)

• RocketSimulation (8-59)

src/views/motor.py (1)

• MotorSimulation (7-73)

src/views/environment.py (1)

• EnvironmentSimulation (13-78)

🪛 checkmake (0.2.2)

Makefile

[warning] 43-43: Target "lint" should be declared PHONY.

(phonydeclared)

🔇 Additional comments (4)

Makefile (1)

6-40: LGTM: Tool variable logic with virtualenv preference.

The conditional logic correctly prioritizes virtualenv-installed tools over system commands, with appropriate fallbacks for pytest and uvicorn using python3 -m. This ensures consistent behavior across different development environments.

src/utils.py (3)

76-76: Correct fix for accessing private Flight method.

The change from obj.__evaluate_post_process() to obj._Flight__evaluate_post_process() correctly uses Python's name mangling convention to access the private method __evaluate_post_process defined in the Flight class. The original code would have failed to call the intended method.

---

128-190: Well-structured refactoring with defensive programming.

The refactored attribute population logic cleanly separates concerns:

• _populate_simulation_attributes: Coordinates the mapping-driven population
• _annotation_keys: Filters annotations by exclusions
• _resolve_attribute_path: Navigates nested attributes safely
• _resolve_attribute_target: Builds nested dict structure
• _copy_missing_attributes: Copies with AttributeError handling

The mapping-based approach at lines 132-142 clearly documents the relationships between simulation classes and their attribute paths.

---

117-126: Confirmed fin sweep/angle fix implemented in src/models/sub/aerosurfaces.py — Fins.get_additional_parameters (lines 47–58) introduces _base_keys and drops sweep_length when both sweep_angle and sweep_length are set.