Open
Conversation
…RocketPy-Team#889) * Fix hard-coded radius value for parachute added mass calculation Calculate radius from cd_s using a typical hemispherical parachute drag coefficient (1.4) when radius is not explicitly provided. This fixes drift distance calculations for smaller parachutes like drogues. Formula: R = sqrt(cd_s / (Cd * π)) Closes RocketPy-Team#860 Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com> Address code review: improve docstrings and add explicit None defaults Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com> Add CHANGELOG entry for PR RocketPy-Team#889 Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com> Update rocket.add_parachute to use radius=None for consistency Changed the default radius from 1.5 to None in the add_parachute method to match the Parachute class behavior. This ensures consistent automatic radius calculation from cd_s across both APIs. Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com> Refactor Parachute class to remove hard-coded radius value and introduce drag_coefficient parameter for radius estimation Fix hard-coded radius value for parachute added mass calculation Calculate radius from cd_s using a typical hemispherical parachute drag coefficient (1.4) when radius is not explicitly provided. This fixes drift distance calculations for smaller parachutes like drogues. Formula: R = sqrt(cd_s / (Cd * π)) Closes RocketPy-Team#860 Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com> Add CHANGELOG entry for PR RocketPy-Team#889 Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com> Refactor Parachute class to remove hard-coded radius value and introduce drag_coefficient parameter for radius estimation MNT: Extract noise initialization to fix pylint too-many-statements in Parachute.__init__ Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com> * Refactor environment method access in controller test for clarity * fix pylint * fix comments * avoid breaking change with drag_coefficient * reafactors Parachute.__init__ method * fix tests --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com> Co-authored-by: Gui-FernandesBR <guilherme_fernandes@usp.br>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
khushalkottaru
force-pushed
the
daily-issue-scout-12924607668829467013
branch
from
ba9ed7b to
169b3e3
Compare
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces 3-DOF flight simulations with weathercocking support, MERRA-2 reanalysis atmosphere model integration, enhanced controller functions with environment parameter, FlightComparator for external flight data validation, rail button bending moment calculations, 7D drag coefficient inputs, ThrustCurve API caching, regular grid interpolation, and various animation and plotting enhancements across the codebase. Changes
Sequence DiagramssequenceDiagram
participant User
participant Flight as Flight<br/>(3-DOF)
participant PointMassRocket
participant Environment
participant Attitude as Attitude<br/>(Quaternion)
User->>Flight: __init__(simulation_mode="3 DOF",<br/>weathercock_coeff=1.0)
Flight->>PointMassRocket: validate point-mass model
Note over Flight: Set u_dot_generalized<br/>= u_dot_generalized_3dof
Flight->>Flight: Simulation Loop (each time step)
Flight->>Environment: Get wind, atmospheric data
Flight->>Flight: Compute 7D drag inputs<br/>(alpha, beta, mach, reynolds, etc.)
Flight->>Flight: u_dot_generalized_3dof(t, state)
Note over Flight: Extract body velocity,<br/>attitude vector
Flight->>Flight: Compute stream velocity<br/>in body frame
Flight->>Flight: Check weathercocking<br/>condition
alt Weathercocking Active
Flight->>Attitude: Compute body-axis<br/>misalignment vs wind
Flight->>Attitude: Compute rotation axis<br/>& angular velocity
Flight->>Attitude: Update quaternion<br/>derivatives
else No Weathercocking
Flight->>Attitude: Set quat derivatives = 0
end
Flight->>Flight: Return generalized<br/>accelerations (13-element)
Flight->>Flight: Integrate for next state
sequenceDiagram
participant User
participant Comparator as FlightComparator
participant Flight as Reference<br/>Flight
participant ExternalData as External Data<br/>(Flight/Dict/CSV)
participant Interpolation
participant Metrics as Error Metrics
User->>Comparator: __init__(flight=reference_flight)
Comparator->>Flight: Validate required attributes<br/>(t_events, x, y, z, etc.)
User->>Comparator: add_data(label, data)
alt Data is Flight
Comparator->>ExternalData: Extract time, variables
else Data is Dict
Comparator->>ExternalData: Store variable mappings
else Data is FlightDataImporter
Comparator->>ExternalData: Load CSV & map variables
end
User->>Comparator: compare(attribute="apogee",<br/>time_range=(0, 100))
Comparator->>Interpolation: Build time grid (1000 pts)
Comparator->>Flight: Interpolate reference at grid
Comparator->>ExternalData: Interpolate each source at grid
Comparator->>Metrics: Compute MAE, RMSE,<br/>max deviation per source
Comparator->>Metrics: Plot reference vs all sources<br/>with residuals
User->>Comparator: summary()
Comparator->>Metrics: Compute key events<br/>(apogee, max_velocity, impact)
Comparator->>Metrics: Format results table
Comparator->>User: Print summary & metrics
sequenceDiagram
participant User
participant Motor as GenericMotor
participant Cache as ~/.rocketpy_cache
participant API as ThrustCurve API
participant Engine as Engine File<br/>(.eng)
User->>Motor: load_from_thrustcurve_api<br/>(name="Bella Lui", no_cache=False)
alt Cache Check
Motor->>Cache: Check if motor cached
Cache-->>Motor: Found cached file
else Cache Miss
Motor->>API: HTTP GET search_motors<br/>?designation={name}
API-->>Motor: Return motor ID
Motor->>API: HTTP GET download<br/>?motor_id={id}
API-->>Motor: Return base64-encoded<br/>.eng data
Motor->>Cache: Decode & write<br/>to ~/.rocketpy_cache
end
Motor->>Engine: Create temp .eng file<br/>from cached/downloaded data
Motor->>Motor: load_from_eng_file(temp_path)
Motor->>Engine: Parse thrust curve, impulse
Engine-->>Motor: GenericMotor instance
Motor->>Engine: Delete temp file
Motor-->>User: Return configured motor
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
rocketpy/plots/environment_plots.py (1)
58-67: De-duplicate wraparound preprocessing logic.The same direction/altitude NaN-break logic is implemented in two methods. Extracting a private helper would keep both plotting paths consistent and easier to maintain.
♻️ Suggested refactor
class _EnvironmentPlots: + `@staticmethod` + def __insert_wrap_nans(directions, altitudes, wrap_threshold=180): + wrap_idx = np.where(np.abs(np.diff(directions)) > wrap_threshold)[0] + 1 + return ( + np.insert(directions, wrap_idx, np.nan), + np.insert(altitudes, wrap_idx, np.nan), + ) + def __wind(self, ax): ... - WRAP_THRESHOLD = 180 # degrees; half the full circle - wrap_indices = np.where(np.abs(np.diff(directions)) > WRAP_THRESHOLD)[0] + 1 - directions = np.insert(directions, wrap_indices, np.nan) - altitudes = np.insert(altitudes, wrap_indices, np.nan) + directions, altitudes = self.__insert_wrap_nans(directions, altitudes) ... def ensemble_member_comparison(self, *, filename=None): ... - WRAP_THRESHOLD = 180 # degrees; half the full circle for i in range(self.environment.num_ensemble_members): ... - wrap_idx = np.where(np.abs(np.diff(dirs)) > WRAP_THRESHOLD)[0] + 1 - dirs = np.insert(dirs, wrap_idx, np.nan) - alts = np.insert(alts, wrap_idx, np.nan) + dirs, alts = self.__insert_wrap_nans(dirs, alts)Also applies to: 322-332
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rocketpy/plots/environment_plots.py` around lines 58 - 67, Duplicate wraparound preprocessing in environment_plots.py should be extracted into a single private helper to keep both plotting paths consistent: implement a helper function (e.g., _break_direction_wrap(directions, altitudes) or _insert_wrap_nan_for_wraparound) that takes 1D direction and altitude arrays, computes wrap indices using the same WRAP_THRESHOLD logic (abs(diff(directions)) > 180), and returns directions and altitudes with NaNs inserted at wrap points; replace the duplicated blocks in the methods that currently build directions/altitudes (the block using self.environment.wind_direction(self.grid) around the top of the file and the similar block around lines 322-332) to call this new helper, ensuring the helper is private and documented briefly so both plotting code paths use identical behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/integration/environment/test_environment.py`:
- Around line 96-131: Reformat the test function
test_wind_plots_wrapping_direction to satisfy ruff formatting rules (run `ruff
format` or your project's formatter) so CI passes; ensure the body and docstring
of test_wind_plots_wrapping_direction and the multi-line set_atmospheric_model
call remain semantically unchanged (keep
example_plain_env.set_atmospheric_model(...), the wind_direction checks
low_dir/high_dir, and the final assertions calling example_plain_env.info() and
example_plain_env.plots.atmospheric_model()) but update whitespace/line breaks
to match ruff's style.
- Around line 96-131: Enhance test_wind_plots_wrapping_direction to verify the
wraparound was handled by inspecting the matplotlib axes after calling
example_plain_env.plots.atmospheric_model(): get the current Axes
(matplotlib.pyplot.gca()), iterate its Line2D objects (ax.lines) and examine
each line's y-data; assert that the plotting code inserted a break at the
360°→0° transition by checking that at least one plotted line has a NaN in its
y-data (or that no consecutive y-values differ by >180°, indicating a segmented
line), so the test fails if the wraparound would produce a continuous horizontal
line across the plot.
---
Nitpick comments:
In `@rocketpy/plots/environment_plots.py`:
- Around line 58-67: Duplicate wraparound preprocessing in environment_plots.py
should be extracted into a single private helper to keep both plotting paths
consistent: implement a helper function (e.g., _break_direction_wrap(directions,
altitudes) or _insert_wrap_nan_for_wraparound) that takes 1D direction and
altitude arrays, computes wrap indices using the same WRAP_THRESHOLD logic
(abs(diff(directions)) > 180), and returns directions and altitudes with NaNs
inserted at wrap points; replace the duplicated blocks in the methods that
currently build directions/altitudes (the block using
self.environment.wind_direction(self.grid) around the top of the file and the
similar block around lines 322-332) to call this new helper, ensuring the helper
is private and documented briefly so both plotting code paths use identical
behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5ef3bea4-d0ec-4af0-919d-b1f80b82b509
📒 Files selected for processing (2)
rocketpy/plots/environment_plots.pytests/integration/environment/test_environment.py
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@jules_handoff.sh`:
- Line 16: The script still contains hard-coded Astropy references (the echo "❌
Not inside a git repo. Run this from your astropy fork directory." and other
occurrences of "astropy", "scout.md", and "astropy/<subpackage>/tests/"); update
those strings and any handoff context output to point to the RocketPy repo and
its correct handoff and test layout (e.g., replace the echo with "❌ Not inside a
git repo. Run this from your RocketPy fork directory.", replace "astropy" tokens
with "RocketPy", change "scout.md" to the project's actual handoff doc name, and
adjust "astropy/<subpackage>/tests/" to "rocketpy/<module>/tests/" or the repo's
real test path) and ensure the same replacements are applied to the other block
noted in the script where the same strings appear.
- Around line 3-9: The usage examples at the top of jules_handoff.sh reference
the wrong script name ("./jules-handoff.sh"); update those examples to use the
actual checked-in filename ("./jules_handoff.sh") (or include both variants) so
the documented commands match the script file name shown in the header; edit the
usage block in jules_handoff.sh to replace "./jules-handoff.sh" occurrences with
"./jules_handoff.sh" (or add a note explaining alternate name) and keep the
three example usages identical otherwise.
- Around line 62-66: The BRANCHES pipeline currently hardcodes grep -v
'origin/main', which fails if the repo default is master; fix by determining the
remote default branch dynamically and excluding it: compute DEFAULT_BRANCH=$(git
symbolic-ref refs/remotes/origin/HEAD | sed 's@refs/remotes/origin/@@') and then
replace the hardcoded grep -v 'origin/main' in the BRANCHES assignment with grep
-v "origin/$DEFAULT_BRANCH" (preserve the rest of the pipeline including sed
's|origin/||' and head -20).
- Around line 115-145: The script currently sets SELECTED and populates
SCOUT_REPORT but only extracts ISSUE_NUMBER from PR_BODY and BRANCH later,
causing empty ISSUE_NUMBER for branches like daily-issue-scout-...; update
jules_handoff.sh to attempt extracting ISSUE_NUMBER from the SCOUT_REPORT
variable immediately after SCOUT_REPORT is populated (before the existing
PR_BODY/BRANCH fallback logic), by parsing SCOUT_REPORT for the upstream issue
number or URL (the same regex used for PR_BODY/BRANCH) and assigning it to
ISSUE_NUMBER when non-empty so the subsequent issue-status and competing-PR
checks can run correctly.
In `@scout_report.md`:
- Around line 1-2: Update the report header string "# Scout Report — 2023-10-24"
to the current PR date by replacing the date fragment "2023-10-24" with
"2026-03-16" so the scout_report.md top-level heading accurately reflects the
report date.
- Around line 95-97: The trailing horizontal rule is being parsed as a setext
underline because there is no blank line before the '---'; update the section
that begins with "### Difficulty" so that there is a single blank line inserted
immediately before the '---' separator (i.e., add an empty line between the
paragraph text and the '---') to ensure the paragraph on the previous line
renders as normal text instead of a heading.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e47bc836-5e77-4a69-a55f-66da4f59bda5
📒 Files selected for processing (9)
CLAUDE.mdjules_handoff.shrocketpy/environment/environment.pyrocketpy/rocket/aero_surface/nose_cone.pyrocketpy/rocket/aero_surface/tail.pyrocketpy/rocket/parachute.pyrocketpy/rocket/rocket.pyrocketpy/simulation/flight.pyscout_report.md
💤 Files with no reviewable changes (1)
- CLAUDE.md
✅ Files skipped from review due to trivial changes (1)
- rocketpy/rocket/aero_surface/tail.py
jules_handoff.sh
Outdated
| REPO_DIR=$(git rev-parse --show-toplevel 2>/dev/null) | ||
| if [ -z "$REPO_DIR" ]; then | ||
| echo "❌ Not inside a git repo." | ||
| echo "❌ Not inside a git repo. Run this from your astropy fork directory." |
There was a problem hiding this comment.
The generated handoff context still points at Astropy instead of RocketPy.
These lines still reference astropy, scout.md, and astropy/<subpackage>/tests/. That sends both the operator and Claude Code to the wrong repo, wrong file, and wrong test layout for this project.
🛠️ Proposed fix
- echo "❌ Not inside a git repo. Run this from your astropy fork directory."
+ echo "❌ Not inside a git repo. Run this from your RocketPy fork directory."
@@
-## Jules Scout Report (scout.md)
+## Jules Scout Report (scout_report.md)
${SCOUT_REPORT:-"(No scout_report.md on this branch — refer to PR description below)"}
@@
-${ISSUE_BODY:-"(Could not fetch issue body — check https://github.com/astropy/astropy/issues)"}
+${ISSUE_BODY:-"(Could not fetch issue body — check https://github.com/${UPSTREAM_REPO}/issues)"}
@@
-1. Read scout.md carefully — Jules has already identified the files and approach.
+1. Read scout_report.md carefully — Jules has already identified the files and approach.
@@
- https://github.com/astropy/astropy/issues/${ISSUE_NUMBER:-""}
+ https://github.com/${UPSTREAM_REPO}/issues/${ISSUE_NUMBER:-""}
@@
-3. Run the existing tests for the relevant subpackage first to establish a baseline:
- \`python -m pytest astropy/<subpackage>/tests/ -x -v\`
+3. Run the existing tests for the relevant area first to establish a baseline:
+ \`python -m pytest tests/ -x -v\`
@@
-4. Implement the fix described in scout.md. Follow astropy standards:
+4. Implement the fix described in scout_report.md. Follow project standards:Also applies to: 225-259
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@jules_handoff.sh` at line 16, The script still contains hard-coded Astropy
references (the echo "❌ Not inside a git repo. Run this from your astropy fork
directory." and other occurrences of "astropy", "scout.md", and
"astropy/<subpackage>/tests/"); update those strings and any handoff context
output to point to the RocketPy repo and its correct handoff and test layout
(e.g., replace the echo with "❌ Not inside a git repo. Run this from your
RocketPy fork directory.", replace "astropy" tokens with "RocketPy", change
"scout.md" to the project's actual handoff doc name, and adjust
"astropy/<subpackage>/tests/" to "rocketpy/<module>/tests/" or the repo's real
test path) and ensure the same replacements are applied to the other block noted
in the script where the same strings appear.
jules_handoff.sh
Outdated
Comment on lines
62
to
66
| BRANCHES=$(git branch -r --sort=-committerdate \ | ||
| | grep 'origin/' \ | ||
| | grep -v "origin/$DEFAULT_BRANCH" \ | ||
| | grep -v 'origin/main' \ | ||
| | sed 's|origin/||' \ | ||
| | head -20) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files | grep -E "^.*\.sh$" | head -20Repository: khushalkottaru/RocketPy
Length of output: 94
🏁 Script executed:
cd . && git branch -r | head -20Repository: khushalkottaru/RocketPy
Length of output: 113
🏁 Script executed:
cd . && git symbolic-ref --quiet --short refs/remotes/origin/HEADRepository: khushalkottaru/RocketPy
Length of output: 80
🏁 Script executed:
cd . && git rev-parse --abbrev-ref origin/HEAD 2>/dev/null || echo "No HEAD reference found"Repository: khushalkottaru/RocketPy
Length of output: 80
🏁 Script executed:
if [ -f "jules_handoff.sh" ]; then
sed -n '55,75p' jules_handoff.sh
else
find . -name "jules_handoff.sh" -type f 2>/dev/null
fiRepository: khushalkottaru/RocketPy
Length of output: 592
Filter out the repo's default branch from the picker.
The code hardcodes grep -v 'origin/main' on line 64, but this repository's default branch is origin/master. This means origin/master is not excluded and appears as a selectable branch in the menu. After the sed removes the origin/ prefix, users can pick master from the menu, which checks out the base branch instead of a work branch—defeating the script's purpose.
🛠️ Proposed fix
+ DEFAULT_REMOTE_BRANCH=$(git symbolic-ref --quiet --short refs/remotes/origin/HEAD 2>/dev/null || true)
BRANCHES=$(git branch -r --sort=-committerdate \
- | grep 'origin/' \
- | grep -v 'origin/main' \
- | sed 's|origin/||' \
+ | sed 's|^ *||' \
+ | grep '^origin/' \
+ | grep -v '^origin/HEAD -> ' \
+ | grep -v "^${DEFAULT_REMOTE_BRANCH}$" \
+ | sed 's|^origin/||' \
| head -20)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@jules_handoff.sh` around lines 62 - 66, The BRANCHES pipeline currently
hardcodes grep -v 'origin/main', which fails if the repo default is master; fix
by determining the remote default branch dynamically and excluding it: compute
DEFAULT_BRANCH=$(git symbolic-ref refs/remotes/origin/HEAD | sed
's@refs/remotes/origin/@@') and then replace the hardcoded grep -v 'origin/main'
in the BRANCHES assignment with grep -v "origin/$DEFAULT_BRANCH" (preserve the
rest of the pipeline including sed 's|origin/||' and head -20).
jules_handoff.sh
Outdated
Comment on lines
+115
to
+145
| # Read the selected issue number from the SELECTED ISSUE line | ||
| SELECTED=$(grep "SELECTED ISSUE:" scout_report.md | grep -oE '[0-9]+' | head -1) | ||
|
|
||
| if [ -z "$SELECTED" ]; then | ||
| echo "" | ||
| echo "👉 Open scout_report.md and fill in the SELECTED ISSUE number, then re-run." | ||
| echo "👉 Open scout_report.md and fill in the SELECTED ISSUE number (1, 2, or 3)" | ||
| echo " Then re-run this script." | ||
| exit 0 | ||
| fi | ||
|
|
||
| echo "✅ Selected issue: #$SELECTED" | ||
|
|
||
| SCOUT_REPORT=$(python3 << PYEOF | ||
| # Extract only the selected issue section | ||
| SCOUT_REPORT=$(python3 - "$SELECTED" << 'PYEOF' | ||
| import sys, re | ||
|
|
||
| selected = "$SELECTED" | ||
| selected = sys.argv[1] | ||
| with open("scout_report.md") as f: | ||
| text = f.read() | ||
|
|
||
| pattern = r"(## Issue " + selected + r" \u2014.*?)(?=## Issue \d|\Z)" | ||
| # Find the section matching "## Issue N" | ||
| pattern = rf"(## Issue {selected} —.*?)(?= | ||
| ## Issue \d|$)" | ||
| match = re.search(pattern, text, re.DOTALL) | ||
| if match: | ||
| print(match.group(1).strip()) | ||
| else: | ||
| print(text) | ||
| print(f"Could not find Issue {selected} in scout_report.md") | ||
| PYEOF | ||
| ) | ||
| echo "✅ Extracted issue $SELECTED from scout_report.md" | ||
| echo "✅ Extracted Issue $SELECTED from scout_report.md" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the file
find . -name "julius_handoff.sh" -o -name "jules_handoff.sh" 2>/dev/null | head -5Repository: khushalkottaru/RocketPy
Length of output: 85
🏁 Script executed:
# Get the total lines to understand file structure
wc -l ./jules_handoff.shRepository: khushalkottaru/RocketPy
Length of output: 89
🏁 Script executed:
# Read the relevant sections: lines 100-220 to capture both code blocks
sed -n '100,220p' ./jules_handoff.sh | cat -nRepository: khushalkottaru/RocketPy
Length of output: 4968
🏁 Script executed:
# Read more lines to see the complete ISSUE_NUMBER extraction logic and subsequent usage
sed -n '150,210p' ./jules_handoff.sh | cat -nRepository: khushalkottaru/RocketPy
Length of output: 3126
🏁 Script executed:
# Find any scout_report files or examples to understand the format
find . -name "*scout*" -type f 2>/dev/null | head -10Repository: khushalkottaru/RocketPy
Length of output: 84
🏁 Script executed:
# Check if there's any documentation about scout_report format
grep -r "scout_report" --include="*.md" --include="*.txt" 2>/dev/null | head -20Repository: khushalkottaru/RocketPy
Length of output: 49
🏁 Script executed:
# Read the scout_report.md file to understand its format
cat ./scout_report.mdRepository: khushalkottaru/RocketPy
Length of output: 6160
🏁 Script executed:
# Verify the complete flow - SCOUT_REPORT definition and ISSUE_NUMBER extraction in context
sed -n '110,170p' ./jules_handoff.sh | cat -nRepository: khushalkottaru/RocketPy
Length of output: 2510
🏁 Script executed:
# Verify line numbers: check what's at absolute line 163
sed -n '160,210p' ./jules_handoff.sh | cat -nRepository: khushalkottaru/RocketPy
Length of output: 2800
🏁 Script executed:
# Verify the proposed fix would correctly extract issue numbers from scout_report.md URLs
# Test the grep pattern against a sample URL
echo "https://github.com/RocketPy-Team/RocketPy/issues/253" | grep -oE '/issues/[0-9]+' | grep -oE '[0-9]+'Repository: khushalkottaru/RocketPy
Length of output: 70
🏁 Script executed:
# Verify that SCOUT_REPORT variable is in scope when ISSUE_NUMBER is extracted
# by checking the bash structure (variables defined in outer scope are available in subsequent if blocks)
grep -n "SCOUT_REPORT=" ./jules_handoff.shRepository: khushalkottaru/RocketPy
Length of output: 140
Extract ISSUE_NUMBER from the selected scout section before falling back to PR body and branch.
You already extract and populate SCOUT_REPORT from scout_report.md (lines 115–145), but the issue-status and competing-PR checks at lines 167–207 only use PR_BODY and BRANCH to find the upstream issue number. For branches like daily-issue-scout-12924607668829467013 that lack recognizable issue references in the PR body, ISSUE_NUMBER remains empty and these critical checks are silently skipped—even though the selected scout report section contains the upstream issue URL.
Proposed fix
SCOUT_REPORT=""
+ISSUE_NUMBER_FROM_SCOUT=""
if [ -f "scout_report.md" ]; then
# Read the selected issue number from the SELECTED ISSUE line
SELECTED=$(grep "SELECTED ISSUE:" scout_report.md | grep -oE '[0-9]+' | head -1)
@@
SCOUT_REPORT=$(python3 - "$SELECTED" << 'PYEOF'
import sys, re
@@
PYEOF
)
+ ISSUE_NUMBER_FROM_SCOUT=$(printf "%s\n" "$SCOUT_REPORT" \
+ | grep -oE '/issues/[0-9]+' \
+ | grep -oE '[0-9]+' \
+ | head -1)
echo "✅ Extracted Issue $SELECTED from scout_report.md"
else
echo "⚠️ No scout_report.md found — will use PR description only"
fi
@@
-ISSUE_NUMBER=""
+ISSUE_NUMBER="$ISSUE_NUMBER_FROM_SCOUT"
@@
- ISSUE_NUMBER=$(echo "$PR_BODY $BRANCH" | grep -oE '#[0-9]+|issues/[0-9]+' | grep -oE '[0-9]+' | head -1)
+ if [ -z "$ISSUE_NUMBER" ]; then
+ ISSUE_NUMBER=$(echo "$PR_BODY $BRANCH" \
+ | grep -oE '#[0-9]+|issues/[0-9]+' \
+ | grep -oE '[0-9]+' \
+ | head -1)
+ fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@jules_handoff.sh` around lines 115 - 145, The script currently sets SELECTED
and populates SCOUT_REPORT but only extracts ISSUE_NUMBER from PR_BODY and
BRANCH later, causing empty ISSUE_NUMBER for branches like
daily-issue-scout-...; update jules_handoff.sh to attempt extracting
ISSUE_NUMBER from the SCOUT_REPORT variable immediately after SCOUT_REPORT is
populated (before the existing PR_BODY/BRANCH fallback logic), by parsing
SCOUT_REPORT for the upstream issue number or URL (the same regex used for
PR_BODY/BRANCH) and assigning it to ISSUE_NUMBER when non-empty so the
subsequent issue-status and competing-PR checks can run correctly.
scout_report.md
Outdated
Comment on lines
+1
to
+2
| --- | ||
| # Scout Report — 2023-10-24 |
There was a problem hiding this comment.
Refresh the report date.
Line 2 still says 2023-10-24, but this PR was opened on March 16, 2026. That makes the daily scout report look stale.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scout_report.md` around lines 1 - 2, Update the report header string "# Scout
Report — 2023-10-24" to the current PR date by replacing the date fragment
"2023-10-24" with "2026-03-16" so the scout_report.md top-level heading
accurately reflects the report date.
scout_report.md
Outdated
Comment on lines
+95
to
+97
| ### Difficulty | ||
| 1 — Requires adding a single instance variable and a straightforward mathematical method to an existing class, plus a basic unit test. | ||
| --- |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd "$(git rev-parse --show-toplevel 2>/dev/null || pwd)" && find . -name "scout_report.md" -type fRepository: khushalkottaru/RocketPy
Length of output: 84
🏁 Script executed:
cat -n scout_report.md | sed -n '90,105p'Repository: khushalkottaru/RocketPy
Length of output: 509
🏁 Script executed:
cat -n scout_report.md | sed -n '95,100p'Repository: khushalkottaru/RocketPy
Length of output: 241
🏁 Script executed:
cat -n scout_report.md | sed -n '95,110p'Repository: khushalkottaru/RocketPy
Length of output: 241
🏁 Script executed:
cat -n scout_report.md | tail -20Repository: khushalkottaru/RocketPy
Length of output: 1612
🏁 Script executed:
python3 << 'EOF'
import markdown
from markdown.extensions import extra
# Test the current structure (without blank line)
test_without_blank = """### Difficulty
1 — Requires adding a single instance variable and a straightforward mathematical method to an existing class, plus a basic unit test.
---"""
# Test the proposed structure (with blank line)
test_with_blank = """### Difficulty
1 — Requires adding a single instance variable and a straightforward mathematical method to an existing class, plus a basic unit test.
---"""
md = markdown.Markdown()
print("=== WITHOUT BLANK LINE ===")
html_without = md.convert(test_without_blank)
print(html_without)
print("\n=== WITH BLANK LINE ===")
md.reset()
html_with = md.convert(test_with_blank)
print(html_with)
print("\n=== COMPARISON ===")
print("Without blank line treats '---' as setext underline?", "<h2>" in html_without or "---" in html_without.lower())
print("With blank line treats '---' as horizontal rule?", "<hr" in html_with)
EOFRepository: khushalkottaru/RocketPy
Length of output: 583
Add a blank line before the final separator.
The "---" on line 97 is currently parsed as a setext underline, causing line 96 to render as a heading (<h2>) instead of normal paragraph text. Adding a blank line before the separator allows it to render correctly as a horizontal rule.
📝 Proposed fix
### Difficulty
1 — Requires adding a single instance variable and a straightforward mathematical method to an existing class, plus a basic unit test.
+
---📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ### Difficulty | |
| 1 — Requires adding a single instance variable and a straightforward mathematical method to an existing class, plus a basic unit test. | |
| --- | |
| ### Difficulty | |
| 1 — Requires adding a single instance variable and a straightforward mathematical method to an existing class, plus a basic unit test. | |
| --- |
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 96-96: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scout_report.md` around lines 95 - 97, The trailing horizontal rule is being
parsed as a setext underline because there is no blank line before the '---';
update the section that begins with "### Difficulty" so that there is a single
blank line inserted immediately before the '---' separator (i.e., add an empty
line between the paragraph text and the '---') to ensure the paragraph on the
previous line renders as normal text instead of a heading.
8068b2d to
2442e39
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (6)
scout_report.md (2)
95-97:⚠️ Potential issue | 🟡 MinorAdd a blank line before the final
---separator.Without the blank line, Line 96 can be parsed as setext heading text instead of a normal paragraph.
🛠️ Proposed fix
### Difficulty 1 — Requires adding a single instance variable and a straightforward mathematical method to an existing class, plus a basic unit test. + ---🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scout_report.md` around lines 95 - 97, Add a blank line between the "### Difficulty" block (the paragraph ending with "1 — Requires adding a single instance variable and a straightforward mathematical method to an existing class, plus a basic unit test.") and the final '---' separator so the paragraph is not parsed as a setext heading; locate the "### Difficulty" heading and insert one empty line immediately before the '---' separator.
2-2:⚠️ Potential issue | 🟡 MinorRefresh the report date in the title.
Line 2 still shows
2023-10-24; it should match this report cycle date.🛠️ Proposed fix
-# Scout Report — 2023-10-24 +# Scout Report — 2026-03-16🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scout_report.md` at line 2, Update the report title line "# Scout Report — 2023-10-24" to reflect the current report cycle date by replacing "2023-10-24" with the correct date for this report; locate the header string in scout_report.md and update that literal timestamp so the document title matches the current cycle.jules_handoff.sh (4)
3-9:⚠️ Potential issue | 🟡 MinorUsage examples reference the wrong filename.
Line 3 and Lines 7-9 use
jules-handoff.sh, but the checked-in script isjules_handoff.sh.🛠️ Proposed fix
-# jules-handoff.sh +# jules_handoff.sh @@ -# ./jules-handoff.sh # interactive branch picker -# ./jules-handoff.sh <branch-name> # direct branch name -# ./jules-handoff.sh <github-pr-url> # paste a Jules PR URL +# ./jules_handoff.sh # interactive branch picker +# ./jules_handoff.sh <branch-name> # direct branch name +# ./jules_handoff.sh <github-pr-url> # paste a Jules PR URL🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@jules_handoff.sh` around lines 3 - 9, Update the usage/help text in the script header to reference the actual filename "jules_handoff.sh" (replace all occurrences of "jules-handoff.sh") so the examples and usage lines (the comment block containing Usage: and the example invocations) show the correct script name; search for the string "jules-handoff.sh" in the file and replace it with "jules_handoff.sh" (e.g., in the header comment and each usage example).
154-165:⚠️ Potential issue | 🟠 Major
ISSUE_NUMBERextraction ignoresSCOUT_REPORTcontent.Line 164 derives issue number from PR body/branch only; this can leave
ISSUE_NUMBERempty fordaily-issue-scout-*branches even when scout data has the upstream issue URL.🛠️ Proposed fix
-ISSUE_NUMBER="" +ISSUE_NUMBER=$(echo "$SCOUT_REPORT" | grep -oE '#[0-9]+|issues/[0-9]+' | grep -oE '[0-9]+' | head -1) @@ - ISSUE_NUMBER=$(echo "$PR_BODY $BRANCH" | grep -oE '#[0-9]+|issues/[0-9]+' | grep -oE '[0-9]+' | head -1) + if [ -z "$ISSUE_NUMBER" ]; then + ISSUE_NUMBER=$(echo "$PR_BODY $BRANCH" | grep -oE '#[0-9]+|issues/[0-9]+' | grep -oE '[0-9]+' | head -1) + fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@jules_handoff.sh` around lines 154 - 165, The ISSUE_NUMBER extraction only inspects PR_BODY and BRANCH; update the logic around ISSUE_NUMBER (the block that uses PR_JSON, PR_BODY and the grep pipeline) to also inspect SCOUT_REPORT content when ISSUE_NUMBER is empty—i.e., attempt to extract an upstream issue URL/number from SCOUT_REPORT using the same patterns ('#[0-9]+' or 'issues/[0-9]+') and assign that value to ISSUE_NUMBER as a fallback so daily-issue-scout-* branches can populate ISSUE_NUMBER from the scout data.
62-66:⚠️ Potential issue | 🟠 MajorBranch picker hardcodes
origin/mainand can include the default branch.Lines 62-66 only exclude
origin/main, so repos withorigin/masterstill show default branch candidates.🛠️ Proposed fix
+ DEFAULT_REMOTE_BRANCH=$(git symbolic-ref --quiet --short refs/remotes/origin/HEAD 2>/dev/null || true) BRANCHES=$(git branch -r --sort=-committerdate \ - | grep 'origin/' \ - | grep -v 'origin/main' \ - | sed 's|origin/||' \ + | sed 's|^ *||' \ + | grep '^origin/' \ + | grep -v '^origin/HEAD -> ' \ + | grep -v "^${DEFAULT_REMOTE_BRANCH}$" \ + | sed 's|^origin/||' \ | head -20) @@ - echo "No branches found other than main." + echo "No branches found other than the default branch."Also applies to: 69-70
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@jules_handoff.sh` around lines 62 - 66, The branch picker currently hardcodes excluding "origin/main" which misses repos whose default branch is "origin/master"; change the logic that builds BRANCHES (and the similar exclusion at lines 69-70) to first determine the remote default branch dynamically (e.g. query origin's HEAD via git for-each-ref or git symbolic-ref/remote show to get refs/remotes/origin/HEAD) and then filter out that dynamically discovered "origin/<default>" instead of the literal "origin/main"; update the BRANCHES pipeline and any subsequent grep/exclude steps to use that computed default-branch name variable.
16-16:⚠️ Potential issue | 🟠 MajorProject context is still hardcoded to Astropy in multiple places.
Line 16 and Lines 225-257 still point users to Astropy/scout.md paths, which breaks RocketPy handoff guidance.
🛠️ Proposed fix
- echo "❌ Not inside a git repo. Run this from your astropy fork directory." + echo "❌ Not inside a git repo. Run this from your RocketPy fork directory." @@ -## Jules Scout Report (scout.md) +## Jules Scout Report (scout_report.md) @@ -${ISSUE_BODY:-"(Could not fetch issue body — check https://github.com/astropy/astropy/issues)"} +${ISSUE_BODY:-"(Could not fetch issue body — check https://github.com/${UPSTREAM_REPO}/issues)"} @@ -1. Read scout.md carefully — Jules has already identified the files and approach. +1. Read scout_report.md carefully — Jules has already identified the files and approach. @@ - https://github.com/astropy/astropy/issues/${ISSUE_NUMBER:-""} + https://github.com/${UPSTREAM_REPO}/issues/${ISSUE_NUMBER:-""} @@ -3. Run the existing tests for the relevant subpackage first to establish a baseline: - `python -m pytest astropy/<subpackage>/tests/ -x -v` +3. Run the existing tests for the relevant area first to establish a baseline: + `python -m pytest tests/ -x -v` @@ -4. Implement the fix described in scout.md. Follow astropy standards: +4. Implement the fix described in scout_report.md. Follow project standards:Also applies to: 225-257
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@jules_handoff.sh` at line 16, The script jules_handoff.sh uses hardcoded "Astropy" paths and messages (e.g., the echo on line 16 and the guidance block around lines 225-257); change those to use a project-aware variable or detection instead of hardcoded text: introduce or reuse a PROJECT_NAME (or derive from git remote/origin) and replace occurrences like "Astropy" and "Astropy/scout.md" with "$PROJECT_NAME" or the computed repo name and correct relative path variables so the messages and link targets point to the current repository (update the echo that prints "Not inside a git repo..." to reference the dynamic project name and adjust the handoff guidance block to use the same variable).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@jules_handoff.sh`:
- Around line 3-9: Update the usage/help text in the script header to reference
the actual filename "jules_handoff.sh" (replace all occurrences of
"jules-handoff.sh") so the examples and usage lines (the comment block
containing Usage: and the example invocations) show the correct script name;
search for the string "jules-handoff.sh" in the file and replace it with
"jules_handoff.sh" (e.g., in the header comment and each usage example).
- Around line 154-165: The ISSUE_NUMBER extraction only inspects PR_BODY and
BRANCH; update the logic around ISSUE_NUMBER (the block that uses PR_JSON,
PR_BODY and the grep pipeline) to also inspect SCOUT_REPORT content when
ISSUE_NUMBER is empty—i.e., attempt to extract an upstream issue URL/number from
SCOUT_REPORT using the same patterns ('#[0-9]+' or 'issues/[0-9]+') and assign
that value to ISSUE_NUMBER as a fallback so daily-issue-scout-* branches can
populate ISSUE_NUMBER from the scout data.
- Around line 62-66: The branch picker currently hardcodes excluding
"origin/main" which misses repos whose default branch is "origin/master"; change
the logic that builds BRANCHES (and the similar exclusion at lines 69-70) to
first determine the remote default branch dynamically (e.g. query origin's HEAD
via git for-each-ref or git symbolic-ref/remote show to get
refs/remotes/origin/HEAD) and then filter out that dynamically discovered
"origin/<default>" instead of the literal "origin/main"; update the BRANCHES
pipeline and any subsequent grep/exclude steps to use that computed
default-branch name variable.
- Line 16: The script jules_handoff.sh uses hardcoded "Astropy" paths and
messages (e.g., the echo on line 16 and the guidance block around lines
225-257); change those to use a project-aware variable or detection instead of
hardcoded text: introduce or reuse a PROJECT_NAME (or derive from git
remote/origin) and replace occurrences like "Astropy" and "Astropy/scout.md"
with "$PROJECT_NAME" or the computed repo name and correct relative path
variables so the messages and link targets point to the current repository
(update the echo that prints "Not inside a git repo..." to reference the dynamic
project name and adjust the handoff guidance block to use the same variable).
In `@scout_report.md`:
- Around line 95-97: Add a blank line between the "### Difficulty" block (the
paragraph ending with "1 — Requires adding a single instance variable and a
straightforward mathematical method to an existing class, plus a basic unit
test.") and the final '---' separator so the paragraph is not parsed as a setext
heading; locate the "### Difficulty" heading and insert one empty line
immediately before the '---' separator.
- Line 2: Update the report title line "# Scout Report — 2023-10-24" to reflect
the current report cycle date by replacing "2023-10-24" with the correct date
for this report; locate the header string in scout_report.md and update that
literal timestamp so the document title matches the current cycle.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7fc0b261-d885-45a8-9fd3-c8cfdb0cb6b5
📒 Files selected for processing (9)
CLAUDE.mdjules_handoff.shrocketpy/environment/environment.pyrocketpy/rocket/aero_surface/nose_cone.pyrocketpy/rocket/aero_surface/tail.pyrocketpy/rocket/parachute.pyrocketpy/rocket/rocket.pyrocketpy/simulation/flight.pyscout_report.md
💤 Files with no reviewable changes (1)
- CLAUDE.md
🚧 Files skipped from review as they are similar to previous changes (5)
- rocketpy/environment/environment.py
- rocketpy/simulation/flight.py
- rocketpy/rocket/aero_surface/tail.py
- rocketpy/rocket/parachute.py
- rocketpy/rocket/rocket.py
khushalkottaru
force-pushed
the
daily-issue-scout-12924607668829467013
branch
from
d3891d1 to
2442e39
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/integration/environment/test_environment.py`:
- Line 5: The import statement "import numpy as np" is unused in the test file;
either remove that unused import or add the intended NaN assertions (e.g., use
np.isnan in the test functions) so the symbol is actually referenced. Locate the
"import numpy as np" line in the test module and either delete it or update the
relevant test functions (the environment tests) to perform the NaN checks using
np.isnan/np.nan_to_num as appropriate and assert the expected behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b785b8bc-9bae-4851-856a-db4fe5789890
📒 Files selected for processing (1)
tests/integration/environment/test_environment.py
Gui-FernandesBR
force-pushed
the
daily-issue-scout-12924607668829467013
branch
from
57d5335 to
bb7b2e3
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
rocketpy/rocket/aero_surface/rail_buttons.py (1)
30-58:⚠️ Potential issue | 🟠 MajorKeep
button_heightkeyword-only and validate it.Adding
button_heightbeforenameandrocket_radiussilently misbinds existing positional calls, and negative heights would feed non-physical bending-moment calculations.Suggested fix
def __init__( self, buttons_distance, angular_position=45, - button_height=None, name="Rail Buttons", rocket_radius=None, + *, + button_height=None, ): @@ super().__init__(name, None, None) self.buttons_distance = buttons_distance self.angular_position = angular_position + if button_height is not None and button_height < 0: + raise ValueError("button_height must be non-negative.") self.button_height = button_height self.name = name self.rocket_radius = rocket_radius @@ `@classmethod` def from_dict(cls, data): return cls( - data["buttons_distance"], - data["angular_position"], - data.get("button_height", None), - data["name"], - data["rocket_radius"], + buttons_distance=data["buttons_distance"], + angular_position=data["angular_position"], + name=data["name"], + rocket_radius=data["rocket_radius"], + button_height=data.get("button_height"), )Also applies to: 121-127
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rocketpy/rocket/aero_surface/rail_buttons.py` around lines 30 - 58, The RailButtons __init__ currently accepts button_height as a positional argument which can break existing calls and allows negative values; change the signature of RailButtons.__init__ so button_height is keyword-only (e.g., place a bare * before button_height) and add validation inside __init__ to raise a ValueError if button_height is not None and is negative; apply the same keyword-only and non-negative validation to any other initializer/factory in this file that accepts button_height (the other RailButtons initialization block).rocketpy/rocket/parachute.py (1)
282-323:⚠️ Potential issue | 🟠 MajorReject unsupported trigger inputs up front.
Any callable other than the 3-arg compatibility case is accepted today, so a 2-arg or 5-arg trigger only fails later when Flight invokes it. Non-string invalid values also hit
trigger.lower()and escape asAttributeErrorinstead of the documentedValueError.Suggested change
if callable(trigger): # work around for having added sensors to parachute triggers # to avoid breaking changes triggerfunc = trigger sig = signature(triggerfunc) if len(sig.parameters) == 3: def triggerfunc(p, h, y, sensors): return trigger(p, h, y) + elif len(sig.parameters) != 4: + raise ValueError( + "Trigger callable must accept 3 or 4 arguments." + ) self.triggerfunc = triggerfunc # Case 2: The parachute is deployed at a given height elif isinstance(trigger, (int, float)): @@ - elif trigger.lower() == "apogee": + elif isinstance(trigger, str) and trigger.lower() == "apogee":🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rocketpy/rocket/parachute.py` around lines 282 - 323, The code currently accepts any callable (including wrong arities) and calls trigger.lower() on non-string triggers causing AttributeError; update the Parachute trigger handling so that if callable(trigger) you inspect signature(trigger) and only accept 3-parameter (wrap to 4 by adding sensors) or 4-parameter callables (use as-is), otherwise raise a ValueError explaining supported arities; change the apogee branch to check isinstance(trigger, str) and compare trigger.lower() == "apogee" so non-string invalid inputs fall through to the final ValueError that describes valid trigger types; reference the existing triggerfunc wrapper logic and the self.triggerfunc assignment when making these checks and errors.tests/unit/mathutils/test_function.py (1)
149-150:⚠️ Potential issue | 🔴 CriticalIncomplete
assertstatement causes syntax error.Line 149 has an incomplete
assert isinstancestatement that's missing the condition. This will cause aSyntaxErrorwhen running the test file.🐛 Proposed fix
- assert isinstance - assert square.integral(2, 4, numerical=True) == -square.integral( + assert isinstance(square.integral(2, 4, numerical=True), float) + assert square.integral(2, 4, numerical=True) == -square.integral(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/mathutils/test_function.py` around lines 149 - 150, The test contains an incomplete statement "assert isinstance" causing a SyntaxError; update the test in tests/unit/mathutils/test_function.py by either removing the stray "assert isinstance" line or completing it to assert a concrete condition about square.integral (e.g., assert the return type or value) so the subsequent assertion comparing square.integral(2, 4, numerical=True) to -square.integral(...) is valid; locate the stray token near the calls to square.integral and fix it by providing the missing arguments/target type or deleting the erroneous line.tests/fixtures/motor/hybrid_fixtures.py (1)
7-15:⚠️ Potential issue | 🟡 MinorUpdate docstring parameter name to match the fixture signature.
The function now accepts
oxidizer_tank, but the docstring still documentsspherical_oxidizer_tank.✏️ Suggested docstring fix
- spherical_oxidizer_tank : rocketpy.LevelBasedTank + oxidizer_tank : rocketpy.LevelBasedTank Example Tank that contains the motor oxidizer. This is a pytest fixture.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/fixtures/motor/hybrid_fixtures.py` around lines 7 - 15, The hybrid_motor fixture's docstring documents the wrong parameter name (mentions spherical_oxidizer_tank) while the function signature uses oxidizer_tank; update the docstring parameter section for hybrid_motor to reference oxidizer_tank (and adjust its description if needed) so the documented parameter name matches the function signature.rocketpy/motors/hybrid_motor.py (1)
357-377:⚠️ Potential issue | 🟠 MajorPersist
only_radial_burnonHybridMotorround-trips.
__init__forwards the flag intoSolidMotor, butHybridMotornever stores it andto_dict()/from_dict()never round-trip it. A motor created withonly_radial_burn=Falsewill deserialize back to the default behavior, which changes the burn geometry.Suggested fix
self.solid = SolidMotor( thrust_source, dry_mass, dry_inertia, nozzle_radius, @@ coordinate_system_orientation, reference_pressure, only_radial_burn, ) + self.only_radial_burn = only_radial_burn @@ "grain_separation": self.grain_separation, "grains_center_of_mass_position": self.grains_center_of_mass_position, "throat_radius": self.throat_radius, + "only_radial_burn": self.only_radial_burn, "positioned_tanks": [ {"tank": tank["tank"], "position": tank["position"]} for tank in self.positioned_tanks ], @@ nozzle_position=data["nozzle_position"], throat_radius=data["throat_radius"], reference_pressure=data.get("reference_pressure"), + only_radial_burn=data.get("only_radial_burn", True), )Also applies to: 655-719
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rocketpy/motors/hybrid_motor.py` around lines 357 - 377, The issue is that HybridMotor.__init__ forwards the only_radial_burn flag into SolidMotor but does not persist it on the HybridMotor instance nor include it in HybridMotor.to_dict()/HybridMotor.from_dict(), so serialization loses that property; fix by adding an instance attribute (e.g., self.only_radial_burn) in HybridMotor.__init__, update HybridMotor.to_dict() to include this key, and update HybridMotor.from_dict() to read the key and pass it back into the constructor (or set the attribute after construction) so round-trips preserve the only_radial_burn setting (refer to the HybridMotor class, its __init__, to_dict, and from_dict methods and the only_radial_burn parameter).rocketpy/simulation/flight.py (1)
722-787:⚠️ Potential issue | 🔴 CriticalParachute trigger logic is duplicated in the same node pass.
The inline parachute loop (Lines 722-783) and
__check_and_handle_parachute_triggers(...)(Line 784 onward) both evaluate triggers/deployments for the same node. This doubles trigger checks/noise sampling and can alter deployment timing.Suggested fix
- for parachute in node.parachutes: - # ... inline trigger/deployment handling ... - self.parachute_events.append([self.t, parachute]) if self.__check_and_handle_parachute_triggers( node, phase, phase_index, node_index ): break # Stop simulation if parachute is deployedAlso applies to: 918-1010
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rocketpy/simulation/flight.py` around lines 722 - 787, The inline parachute loop duplicates the same trigger/deployment work done by __check_and_handle_parachute_triggers (double-sampling noise via __calculate_and_save_pressure_signals and performing phase changes), so remove the inline block that iterates node.parachutes (the loop using __calculate_and_save_pressure_signals, parachute.triggerfunc checks, flight_phases.add_phase calls, parachute_events.append, and phase.solver.status = "finished") and consolidate all deployment logic into __check_and_handle_parachute_triggers so triggers are evaluated exactly once per node; if any deployment-specific side-effects (setting parachute_* attributes, callbacks list construction, phase.time_nodes modifications) currently live only in the inline loop, move them into __check_and_handle_parachute_triggers (or have that method call a helper that performs those exact steps) so behavior and timing remain identical but without duplicated noise sampling and checks.
🟠 Major comments (20)
pyproject.toml-68-68 (1)
68-68:⚠️ Potential issue | 🟠 MajorRemove the upper version constraint for
contextily.The constraint
python_version < '3.14'is incorrect.contextilyv1.7.0+ (released November 2025) is fully compatible with Python 3.14 and declares it in its PyPI classifiers. Remove the upper bound to allow Python 3.14 support:"contextily>=1.0.0",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pyproject.toml` at line 68, The dependency entry for "contextily" in pyproject.toml incorrectly restricts installation to Python versions below 3.14; update the dependency requirement for the "contextily" package by removing the python_version upper bound so the project uses a plain version specifier for contextily (i.e., change the existing "contextily>=1.0.0; python_version < '3.14'" entry to a simple "contextily>=1.0.0" requirement) so Python 3.14-compatible releases are allowed.tests/fixtures/controller/controller_fixtures.py-29-40 (1)
29-40:⚠️ Potential issue | 🟠 MajorApply in-band deployment changes instead of discarding them.
The
elsebranch on Lines 38-39 resets the value to the current deployment level whenever the requested change is already within the rate limit, so this controller only moves when it saturates.Suggested fix
new_deployment_level = ( air_brakes.deployment_level + 0.1 * vz + 0.01 * previous_vz**2 ) - if new_deployment_level > air_brakes.deployment_level + 0.2 / sampling_rate: - new_deployment_level = air_brakes.deployment_level + 0.2 / sampling_rate - elif ( - new_deployment_level < air_brakes.deployment_level - 0.2 / sampling_rate - ): - new_deployment_level = air_brakes.deployment_level - 0.2 / sampling_rate - else: - new_deployment_level = air_brakes.deployment_level + max_change = 0.2 / sampling_rate + new_deployment_level = max( + air_brakes.deployment_level - max_change, + min( + new_deployment_level, + air_brakes.deployment_level + max_change, + ), + ) air_brakes.deployment_level = new_deployment_level🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/fixtures/controller/controller_fixtures.py` around lines 29 - 40, The code erroneously resets small (within-rate-limit) changes by assigning new_deployment_level = air_brakes.deployment_level in the else branch, so the controller only moves when saturated; remove that reset (or simply do nothing in the else) so that air_brakes.deployment_level is set to the computed new_deployment_level when the change is within the rate limits—update the logic around new_deployment_level and the final assignment to air_brakes.deployment_level (referencing new_deployment_level, air_brakes.deployment_level, sampling_rate, vz, previous_vz) so in-band changes are applied instead of discarded.tests/fixtures/controller/controller_fixtures.py-21-25 (1)
21-25:⚠️ Potential issue | 🟠 MajorGuard the initial callback before reading
state_history[-1].Line 23 dereferences the previous state before the early return on Line 24, so the first controller tick can raise
IndexErrorinstead of returningNone.Suggested fix
- z = state[2] - vz = state[5] - previous_vz = state_history[-1][5] if time < 3.9: return None + z = state[2] + vz = state[5] + previous_vz = state_history[-1][5] if state_history else vz🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/fixtures/controller/controller_fixtures.py` around lines 21 - 25, The code reads previous_vz = state_history[-1][5] before early-returning when time < 3.9, which can raise IndexError on the first tick; fix by guarding that access—either check that state_history is non-empty (e.g., if not state_history: return None) before computing previous_vz, or move the previous_vz assignment after the early return so the function returns None when time < 3.9 without dereferencing state_history; update the block that references time, state_history, previous_vz, state, and vz accordingly.tests/unit/simulation/test_monte_carlo_plots_background.py-17-19 (1)
17-19:⚠️ Potential issue | 🟠 MajorMove
pytest.importorskipinto tests that actually require contextily.The module-level skip prevents
test_ellipses_background_raises_error_when_contextily_not_installedfrom running when contextily is absent. This test mocks the missing dependency to verify error handling—it doesn't need contextily to actually be missing. Move the skip to individual tests that depend on contextily being installed (or use local skips like line 363), allowing tests that work without it (e.g., withbackground=None) to run unconditionally.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/simulation/test_monte_carlo_plots_background.py` around lines 17 - 19, Remove the module-level pytest.importorskip call so tests can run when contextily is absent and instead add per-test skips only to tests that truly require contextily; specifically, delete the top-level pytest.importorskip and add pytest.importorskip or pytest.mark.skipif inside or on tests that rely on contextily (e.g., the tests that use contextily in rendering/plotting), while leaving test_ellipses_background_raises_error_when_contextily_not_installed and tests using background=None unskipped so they can mock the missing dependency and validate error handling.tests/unit/simulation/test_monte_carlo_plots_background.py-71-127 (1)
71-127:⚠️ Potential issue | 🟠 MajorMock the background map tile fetching in these unit tests.
Tests at lines 71-127 (
test_ellipses_background_types_display_successfullyandtest_ellipses_image_takes_precedence_over_background) and lines 202-219 and 223-243 make calls tocontextily.bounds2img()without mocking it, which means they attempt real network requests to tile providers. These tests will fail due to network unavailability, provider outages, or rate limiting rather than testing the plotting logic itself.Use the mocking pattern already established in
test_ellipses_background_handles_bounds2img_failures(lines 338–391): mock contextily with a MagicMock that preserves the real providers but stubsbounds2imgto return mock tile data.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/simulation/test_monte_carlo_plots_background.py` around lines 71 - 127, The tests test_ellipses_background_types_display_successfully and test_ellipses_image_takes_precedence_over_background call MockMonteCarlo.plots.ellipses which triggers contextily.bounds2img network tile fetches; mock contextily the same way as in test_ellipses_background_handles_bounds2img_failures by patching contextily with a MagicMock that preserves the real providers (e.g., contextily.providers) but replaces bounds2img to return a small fake tile array (and appropriate transform/extent) so no network request occurs; apply this patch in both tests (and the other mentioned tests at lines 202-219 and 223-243) so image reading logic (imageio.imread) and background selection behavior are exercised without real tile downloads.tests/unit/simulation/test_monte_carlo.py-115-123 (1)
115-123:⚠️ Potential issue | 🟠 MajorSeed the width comparison to avoid a flaky bootstrap test.
These two assertions compare independently resampled intervals. Without
random_state, the monotonicity check is probabilistic rather than deterministic. Both calls toestimate_confidence_interval()usescipy.stats.bootstrapinternally, which requires seeding for reproducible results.Suggested fix
- ci_90 = mc.estimate_confidence_interval("apogee", confidence_level=0.90) + ci_90 = mc.estimate_confidence_interval( + "apogee", confidence_level=0.90, random_state=0 + ) width_90 = ci_90.high - ci_90.low - ci_99 = mc.estimate_confidence_interval("apogee", confidence_level=0.99) + ci_99 = mc.estimate_confidence_interval( + "apogee", confidence_level=0.99, random_state=0 + ) width_99 = ci_99.high - ci_99.low🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/simulation/test_monte_carlo.py` around lines 115 - 123, The test test_estimate_confidence_interval_increases_width_with_higher_confidence_level is flaky because estimate_confidence_interval (used by MockMonteCarlo) calls scipy.stats.bootstrap without a seed; fix by passing a fixed random_state to both calls so bootstrap is deterministic (e.g., call mc.estimate_confidence_interval("apogee", confidence_level=0.90, random_state=0) and mc.estimate_confidence_interval("apogee", confidence_level=0.99, random_state=0)); this keeps the Monte Carlo resampling reproducible and makes the width_99 > width_90 assertion stable.rocketpy/mathutils/function.py-207-273 (1)
207-273:⚠️ Potential issue | 🟠 MajorSingle-axis regular-grid CSVs silently downgrade to spline.
from_regular_grid_csv()can returninterpolation="regular_grid"for a one-variable CSV, but__validate_interpolation()rejects that mode fordom_dim == 1and rewrites it to"spline". Header-based 1-column tables will therefore interpolate differently from the loader's contract. Either allowregular_gridin 1-D or skip this path for single-axis CSVs.Also applies to: 4139-4173
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rocketpy/mathutils/function.py` around lines 207 - 273, from_regular_grid_csv currently produces interpolation="regular_grid" for single-variable CSVs but __validate_interpolation rewrites regular_grid to "spline" for dom_dim == 1, causing a silent downgrade; to fix, short-circuit from_regular_grid_csv to avoid claiming regular_grid for 1-D inputs by returning None when len(variable_names) == 1 (or otherwise skip the regular-grid path for single-axis CSVs), so callers fall back to the loader that produces the intended spline behavior; reference the from_regular_grid_csv classmethod and the __validate_interpolation dom_dim==1 rule when making this change.rocketpy/tools.py-761-771 (1)
761-771:⚠️ Potential issue | 🟠 MajorNormalize longitude wrapping around the antimeridian.
Both helpers assume longitudes are safely comparable with plain
min/maxand< origin_lon, which breaks extents that cross ±180°. A site near the dateline will produce nearly world-wide WGS84 bounds here and flipped local X signs on the reverse conversion. Normalize longitude deltas around the origin before deriving west/east and x signs.Also applies to: 810-830
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rocketpy/tools.py` around lines 761 - 771, The bounding-box logic collects longitudes in req_lons then returns min/max directly, which breaks when extents cross the antimeridian; instead compute each longitude delta relative to origin_lon (use normalized angular difference: (lon - origin_lon + 180) % 360 - 180) before deciding west/east and before calling inverted_haversine or comparing min/max, then convert the chosen min/max deltas back to absolute longitudes (add origin_lon and re-normalize to -180..180). Apply the same delta-normalization pattern to the similar block around lines 810-830 and ensure functions/invocations mentioned here (corners_xy loop, inverted_haversine, origin_lon, req_lons/req_lats) use the normalized deltas for comparisons and final returned bounds.rocketpy/mathutils/function.py-562-568 (1)
562-568:⚠️ Potential issue | 🟠 MajorClamp the left boundary before using
bisect_left.At the exact lower bound,
bisect_leftreturns0, sox_data[x_interval - 1]reads the last sample and computes the segment across the whole dataset. Exactx_minevaluations return the wrong value.Suggested change
- def linear_interpolation(x, x_min, x_max, x_data, y_data, coeffs): # pylint: disable=unused-argument - x_interval = bisect_left(x_data, x) + def linear_interpolation(x, x_min, x_max, x_data, y_data, coeffs): # pylint: disable=unused-argument + x_interval = max(bisect_left(x_data, x), 1) x_left = x_data[x_interval - 1] y_left = y_data[x_interval - 1] dx = float(x_data[x_interval] - x_left) dy = float(y_data[x_interval] - y_left) return (x - x_left) * (dy / dx) + y_left🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rocketpy/mathutils/function.py` around lines 562 - 568, The linear_interpolation function can index x_data[x_interval - 1] when bisect_left returns 0 for x == x_min; change the logic in linear_interpolation to clamp or handle the left boundary: if x <= x_min (or if x_interval == 0) return y_data[0] (or set x_interval = 1 before using x_interval - 1) so you never read the last sample; update linear_interpolation to use that guarded x_interval when computing x_left, y_left, dx, dy.rocketpy/rocket/parachute.py-223-245 (1)
223-245:⚠️ Potential issue | 🟠 MajorDon't treat
0as "unset", and validate inferred geometry before the square root.
height or radiusoverwrites an explicitheight=0, anddrag_coefficient <= 0orcd_s < 0currently letnp.sqrtcreateinf/nangeometry silently. Useis not Nonefor defaults and raiseValueErrorbefore inferring the radius.Suggested change
def __resolve_radius(self, radius, cd_s, drag_coefficient): """Resolves parachute radius from input or aerodynamic relation.""" if radius is not None: return radius + if drag_coefficient <= 0: + raise ValueError("drag_coefficient must be greater than 0.") + if cd_s < 0: + raise ValueError("cd_s must be non-negative.") + # cd_s = Cd * S = Cd * pi * R^2 => R = sqrt(cd_s / (Cd * pi)) return np.sqrt(cd_s / (drag_coefficient * np.pi)) def __resolve_height(self, height, radius): """Resolves parachute height defaulting to radius when not provided.""" - return height or radius + return radius if height is None else height🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rocketpy/rocket/parachute.py` around lines 223 - 245, The code treats 0 as unset and allows invalid inputs to produce nan/inf; update __resolve_height to use "if height is not None: return height" (so height=0 is kept) and add input validation in __resolve_radius: if radius is not None validate radius >= 0 (raise ValueError if negative), otherwise validate drag_coefficient > 0 and cd_s >= 0 and raise ValueError with a clear message before calling np.sqrt; compute R = sqrt(cd_s / (drag_coefficient * np.pi)) only after these checks.rocketpy/plots/flight_plots.py-446-452 (1)
446-452:⚠️ Potential issue | 🟠 MajorFix incorrect attribute name in three rail-button bending-moment fallback branches.
The code uses
self.flight.tFinalwhich doesn't exist on theFlightclass (the canonical attribute ist_final). This causesAttributeErrorin edge cases whereout_of_rail_timeis not positive.Affected lines
Lines 451, 508, and 541 in
flight_plots.pyall need the same fix:- else self.flight.tFinal + else self.flight.t_final🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rocketpy/plots/flight_plots.py` around lines 446 - 452, Replace the incorrect attribute self.flight.tFinal with the correct Flight attribute self.flight.t_final in the three fallback branches that set x-limits for rail-related plots (the calls using ax1.set_xlim that reference self.flight.out_of_rail_time); update each occurrence so the ternary/fallback uses self.flight.t_final instead of tFinal to avoid AttributeError when out_of_rail_time <= 0.rocketpy/environment/environment.py-2734-2743 (1)
2734-2743:⚠️ Potential issue | 🟠 MajorSame case mismatch for ensemble check.
Line 2734 checks
atmospheric_model == "ensemble"butatmospheric_model_typestores the original case"Ensemble".🐛 Proposed fix
- if atmospheric_model == "ensemble": + if atmospheric_model == "Ensemble":Or use case-insensitive comparison:
- if atmospheric_model == "ensemble": + if atmospheric_model.lower() == "ensemble":🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rocketpy/environment/environment.py` around lines 2734 - 2743, The condition uses atmospheric_model == "ensemble" while the original variable atmospheric_model_type holds "Ensemble" (different case); update the check in the block that assigns env.level_ensemble, env.height_ensemble, etc. to compare case-insensitively (e.g., normalize atmospheric_model_type.lower() == "ensemble") or compare against the exact "Ensemble" value, ensuring the ensemble branch (the block setting env.*_ensemble and env.num_ensemble_members) actually executes when the atmospheric model is Ensemble.rocketpy/environment/environment.py-2725-2732 (1)
2725-2732:⚠️ Potential issue | 🟠 MajorCase mismatch in
from_dictconditional check.The
atmospheric_model_typeis stored with its original case (e.g.,"Forecast","Ensemble") at line 1277, but the condition on line 2725 checks against lowercase values ("windy","forecast","reanalysis","ensemble"). This will never match, causing the block to be skipped for valid Forecast/Reanalysis/Ensemble types.🐛 Proposed fix
- if atmospheric_model in ("windy", "forecast", "reanalysis", "ensemble"): + if atmospheric_model in ("windy", "Forecast", "Reanalysis", "Ensemble"):Alternatively, normalize the comparison:
- if atmospheric_model in ("windy", "forecast", "reanalysis", "ensemble"): + if atmospheric_model.lower() in ("windy", "forecast", "reanalysis", "ensemble"):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rocketpy/environment/environment.py` around lines 2725 - 2732, The conditional in the from_dict deserialization that checks atmospheric_model against ("windy", "forecast", "reanalysis", "ensemble") is case-sensitive and won't match values saved with original casing (e.g., "Forecast", "Ensemble"); update the check in the from_dict path that sets env.atmospheric_model_init_date / end_date / interval / init_lat / end_lat / init_lon / end_lon to perform a case-insensitive comparison (e.g., compare atmospheric_model.lower() to the lowercase set) or compare against the exact stored variants (e.g., "Forecast", "Reanalysis", "Ensemble"), ensuring you reference the same atmospheric_model variable used earlier in the class (the attribute/property holding atmospheric_model_type) so the block executes for valid Forecast/Reanalysis/Ensemble types.rocketpy/motors/point_mass_motor.py-29-32 (1)
29-32:⚠️ Potential issue | 🟠 Major
propellant_final_massis exposed here but not actually supported.The validation says callers can omit
burn_timeif they providepropellant_final_mass, but this class never usespropellant_final_massin its mass model andsuper().__init__()still requiresburn_timefor constant/callable thrust. That makes this public API branch impossible to use as written.Also applies to: 45-58
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rocketpy/motors/point_mass_motor.py` around lines 29 - 32, The validation branch wrongly advertises support for propellant_final_mass though PointMassMotor.__init__ never uses it and super().__init__ still requires burn_time for constant/callable thrust; either remove propellant_final_mass from the API/validation or implement it end-to-end. Fix by updating PointMassMotor.__init__ (and the conditional around super().__init__) to either (A) drop propellant_final_mass from the conditional/parameters and require burn_time for constant/callable thrust, or (B) accept propellant_final_mass, compute/derive burn_time or pass propellant_final_mass into the mass model and forward it to super().__init__ so the mass evolution uses it. Ensure the unique symbols touched are PointMassMotor.__init__ and the call to super().__init__ (and any mass model methods) so callers and validation are consistent.tests/unit/motors/test_genericmotor.py-280-338 (1)
280-338:⚠️ Potential issue | 🟠 MajorIsolate this test from the real cache directory.
This test exercises mocked API responses, but it still uses the live
rocketpy.motors.motor.CACHE_DIR. A pre-existing~/.rocketpy_cache/M1670.eng.b64orFakeMotor.eng.b64on the runner can bypass the mocks and make the assertions flaky.Suggested fix
- motor = type(generic_motor).load_from_thrustcurve_api("M1670") + motor = type(generic_motor).load_from_thrustcurve_api("M1670", no_cache=True) @@ - type(generic_motor).load_from_thrustcurve_api("FakeMotor") + type(generic_motor).load_from_thrustcurve_api("FakeMotor", no_cache=True)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/motors/test_genericmotor.py` around lines 280 - 338, The test test_load_from_thrustcurve_api is using the real rocketpy.motors.motor.CACHE_DIR which can make the mocked API flaky; patch the module-level CACHE_DIR used by GenericMotor.load_from_thrustcurve_api to a temporary directory (e.g., monkeypatch.setattr("rocketpy.motors.motor.CACHE_DIR", tmp_path / "cache") or similar) at the start of the test, ensure the dir is empty (create it) and then run the existing monkeypatched requests calls and assertions so cached files cannot be picked up from the real home cache.rocketpy/motors/solid_motor.py-544-563 (1)
544-563:⚠️ Potential issue | 🟠 MajorFix the radial-burn Jacobian.
The Jacobian in the
only_radial_burnbranch does not matchdr/dt = -volume_diff / (2π r h). In particular, Line 552 zeros∂(dr/dt)/∂heven thoughdr/dtstill depends ongrain_height. Passing an inconsistent Jacobian to LSODA can distort the solution in radial-only mode.Suggested fix
if self.only_radial_burn: - factor = volume_diff / ( - 2 * np.pi * (grain_inner_radius * grain_height) ** 2 - ) - - inner_radius_derivative_wrt_inner_radius = factor * ( - grain_height - 2 * grain_inner_radius - ) - inner_radius_derivative_wrt_height = 0 + inner_radius_derivative_wrt_inner_radius = volume_diff / ( + 2 * np.pi * grain_height * grain_inner_radius**2 + ) + inner_radius_derivative_wrt_height = volume_diff / ( + 2 * np.pi * grain_inner_radius * grain_height**2 + ) height_derivative_wrt_inner_radius = 0 height_derivative_wrt_height = 0🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rocketpy/motors/solid_motor.py` around lines 544 - 563, The radial-only Jacobian is wrong: in the only_radial_burn branch compute dr/dt = -volume_diff / (2*pi*grain_inner_radius*grain_height) and replace the incorrect factor-based derivatives with the correct partials; set inner_radius_derivative_wrt_inner_radius = volume_diff / (2*pi * grain_height * grain_inner_radius**2) and inner_radius_derivative_wrt_height = -volume_diff / (2*pi * grain_inner_radius * grain_height**2), keep the height derivatives zero, and return the Jacobian matrix using these values (refer to the only_radial_burn branch, variables volume_diff, grain_inner_radius, grain_height and the returned 2x2 matrix).rocketpy/motors/motor.py-1977-1998 (1)
1977-1998:⚠️ Potential issue | 🟠 MajorAdd timeouts to the ThrustCurve API calls.
Both
requests.get(...)calls are currently unbounded. In library code that can hang notebooks, CI jobs, or any caller indefinitely when the upstream stalls.Suggested fix
- response = requests.get(f"{base_url}/search.json", params={"commonName": name}) + response = requests.get( + f"{base_url}/search.json", + params={"commonName": name}, + timeout=(5, 30), + ) @@ - dl_response = requests.get( + dl_response = requests.get( f"{base_url}/download.json", params={"motorIds": motor_id, "format": "RASP", "data": "file"}, + timeout=(5, 30), )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rocketpy/motors/motor.py` around lines 1977 - 1998, The two unbounded HTTP calls using requests.get (the one assigning response from f"{base_url}/search.json" and the dl_response from f"{base_url}/download.json" in rocketpy.motors.motor) must include a timeout to avoid hanging; add a timeout argument (e.g., timeout=10) to both requests.get calls or wire a configurable timeout value (constant or function/class parameter) into these calls, and propagate that configuration where the motor lookup/download is invoked so callers can override if needed; ensure you update any related callers/tests to pass the timeout when required.rocketpy/simulation/flight_comparator.py-549-555 (1)
549-555:⚠️ Potential issue | 🟠 MajorCompute external key events on each source’s own timeline.
Every external series is sampled on
[0, self.flight.t_final]. If a log/simulation ends earlier or later than the reference flight, apogee can be clipped andimpact_velocityis read at the wrong instant (t_grid[-1]of the reference), so these error metrics can be badly wrong.A safer approach is to keep each source’s original time span when it is added, then derive apogee/max-speed/impact from that source’s own samples instead of forcing everything onto the reference flight duration.
Also applies to: 581-680
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rocketpy/simulation/flight_comparator.py` around lines 549 - 555, The current code forces every external series onto a shared t_grid built from self.flight.t_final (t_grid, DEFAULT_GRID_POINTS), which can clip or mis-evaluate apogee/impact for sources with different durations; change the logic to use each source's own time span when sampling: when adding a data source (self.data_sources entries), store or compute that source's native time vector (e.g., source_t_grid) and use it to evaluate altitude/velocity functions (alt_func(source_t_grid)) and cache per-source results in altitude_cache keyed by label; update subsequent computations that read apogee, max-speed, and impact (the blocks around lines 581-680) to derive metrics from each source's own sampled times rather than the shared reference t_grid/self.flight.t_final.rocketpy/simulation/flight.py-506-507 (1)
506-507:⚠️ Potential issue | 🟠 MajorNew init parameters are not preserved by serialization round-trip.
simulation_modeandweathercock_coeffare now constructor state (Lines 506-507, 628-630), but they are not included into_dict/from_dictin this file. Instances recreated from dict silently fall back to defaults.Suggested fix (outside selected lines)
def to_dict(self, **kwargs): data = { + "simulation_mode": self.simulation_mode, + "weathercock_coeff": self.weathercock_coeff, ... } `@classmethod` def from_dict(cls, data): return cls( ... + simulation_mode=data.get("simulation_mode", "6 DOF"), + weathercock_coeff=data.get("weathercock_coeff", 0.0), )Also applies to: 628-630
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rocketpy/simulation/flight.py` around lines 506 - 507, The new constructor parameters simulation_mode and weathercock_coeff are not serialized, so update Flight.to_dict to include keys for "simulation_mode" and "weathercock_coeff" (serialize the current values from self.simulation_mode and self.weathercock_coeff) and update Flight.from_dict to read those keys and pass them into the Flight constructor (or set them after instantiation) so that a dict round-trip preserves these fields; locate the Flight class __init__ signature (where simulation_mode and weathercock_coeff are defined) and the to_dict/from_dict methods and add handling for those two fields accordingly.rocketpy/simulation/flight.py-1218-1222 (1)
1218-1222:⚠️ Potential issue | 🟠 Major
impact_velocitystores vertical component, not speed magnitude.Line 1221 assigns
self.impact_state[5](vz), butimpact_velocityis documented as velocity magnitude. This yields incorrect output.Suggested fix
- self.impact_velocity = self.impact_state[5] + self.impact_velocity = float(np.linalg.norm(self.impact_state[3:6]))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rocketpy/simulation/flight.py` around lines 1218 - 1222, The code currently sets self.impact_velocity = self.impact_state[5] which is only the vertical component (vz); change it to compute the speed magnitude from the velocity components in impact_state (indices 3,4,5) — e.g., replace the assignment with a magnitude calculation like sqrt(vx**2+vy**2+vz**2) or use numpy.linalg.norm(self.impact_state[3:6]) so impact_velocity represents total speed at impact; update any imports if needed and ensure references remain self.impact_state and self.impact_velocity.
🟡 Minor comments (10)
docs/user/three_dof_simulation.rst-678-681 (1)
678-681:⚠️ Potential issue | 🟡 MinorQualify benchmark/accuracy statements as example-specific.
Line 678-Line 681 read as broadly guaranteed outcomes. Please frame them as results from this specific setup to avoid overpromising in general use.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/user/three_dof_simulation.rst` around lines 678 - 681, The four summary bullets "**3-DOF is 5-7x faster**", "**Apogee prediction**", "Weathercocking", and "**Higher weathercock_coeff**" should be rewritten to state they are results from this specific simulation setup/example rather than general guarantees; update those lines to include phrases like "In this example/setup," or "For the configuration used here," and qualify ranges (e.g., "about 5–7× faster in this case") and accuracy (e.g., "apogee prediction was within 1–3% for this run") while keeping the original concepts and keywords (3-DOF, apogee prediction, weathercocking, weathercock_coeff) so readers can locate the claims easily.docs/user/mrs.rst-30-32 (1)
30-32:⚠️ Potential issue | 🟡 MinorFix grammar in the Monte Carlo sentence.
Line 30 currently reads “need new a Monte Carlo simulation,” which is awkward/incorrect phrasing.
✏️ Suggested wording
-Now, imagine that you ran and saved the Monte Carlo simulations. Later, you need new a -Monte Carlo simulation with different probability distributions that are somewhat close +Now, imagine that you ran and saved the Monte Carlo simulations. Later, you need a new +Monte Carlo simulation with different probability distributions that are somewhat close🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/user/mrs.rst` around lines 30 - 32, Replace the awkward phrase "need new a Monte Carlo simulation" with correct wording such as "need a new Monte Carlo simulation" in the sentence containing the Monte Carlo example (look for the exact phrase "need new a Monte Carlo simulation") so the sentence reads smoothly: "Later, you need a new Monte Carlo simulation with different probability distributions..." ensuring article ordering is fixed and grammar is correct.CHANGELOG.md-35-45 (1)
35-45:⚠️ Potential issue | 🟡 MinorClean up the
Unreleasedentries before merging.This block mixes a test-only item into the public changelog and also has several malformed references (
[RocketPy-Team#914] (...),[RocketPy-Team#907] (...),[[RocketPy-Team#906]...), plus Line 44's displayed PR number and target disagree (#891vs#897). As written, some release-note links will render broken.Also applies to: 61-63
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CHANGELOG.md` around lines 35 - 45, Remove or move the test-only entry "TST: Add acceptance tests for 3DOF flight simulation based on Bella Lui rocket [`#914`] (https://github.com/RocketPy-Team/RocketPy/pull/914_" out of the public Unreleased list; fix all malformed markdown link syntax (e.g. change "[`#914`] (https://.../pull/914_" to "[`#914`](https://.../pull/914)") and remove stray characters/extra spaces in entries like "TST: ..." and "MNT: net thrust addition to 3 dof in flight class [`#907`] (https://...)" so they use the form [`#NNN`](URL). Correct the mismatched PR label in "ENH: Implement Bootstrapping for Confidence Interval Estimation [`#891`](https://github.com/.../pull/897)" so the displayed PR number matches the URL (either change [`#891`] to [`#897`] or update the URL to pull/891), and scan the rest of the Unreleased block (including the similar issues around the entries for Rail button bending moments and Monte Carlo changes) to ensure every entry uses consistent "[`#NNN`](PR URL)" formatting and accurate PR numbers.docs/user/motors/liquidmotor.rst-111-112 (1)
111-112:⚠️ Potential issue | 🟡 MinorUse the exact parameter name
dry_inertiain the caution block.The current token
dry inertiacan be mistaken for plain text instead of the constructor argument.✏️ Suggested docs fix
- - ``dry inertia`` is defined in relation to the **center of dry mass**, and + - ``dry_inertia`` is defined in relation to the **center of dry mass**, and🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/user/motors/liquidmotor.rst` around lines 111 - 112, The caution currently uses the plain token ``dry inertia`` which can be mistaken for text; update the caution to reference the exact constructor parameter name `dry_inertia` (including backticks or inline code formatting consistent with surrounding rst) so it clearly denotes the parameter used in the constructor/API (e.g., mention `dry_inertia` in the caution about the center of dry mass vs coordinate origin).docs/user/motors/tanks.rst-144-145 (1)
144-145:⚠️ Potential issue | 🟡 MinorFix wording typo in geometry description.
The phrase should be singular: “2 m length”.
✏️ Suggested wording fix
-This defines a cylindrical tank with a 2 m lengths (from -1 m to 1 m) and a +This defines a cylindrical tank with a 2 m length (from -1 m to 1 m) and a🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/user/motors/tanks.rst` around lines 144 - 145, The sentence describing the cylindrical tank wording contains a plural typo — change “2 m lengths (from -1 m to 1 m) and a” to the singular phrasing “2 m length (from -1 m to 1 m) and a” so the geometry description reads “This defines a cylindrical tank with a 2 m length (from -1 m to 1 m) and a constant radius of 0.1 m.” and update any identical occurrences of the same phrase elsewhere in the document.rocketpy/plots/tank_plots.py-184-247 (1)
184-247:⚠️ Potential issue | 🟡 MinorValidate
fpsbefore building the animation interval.This method also does
1000 / fps, sofps=0crashes and negative values generate a nonsensical interval.⏱️ Suggested guard
def animate_fluid_volume(self, filename=None, fps=30): """Animates the liquid and gas volumes inside the tank as a function of time. @@ + if fps <= 0: + raise ValueError("fps must be greater than 0") + t_start, t_end = self.flux_time times = np.linspace(t_start, t_end, 200)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rocketpy/plots/tank_plots.py` around lines 184 - 247, The animate_fluid_volume method currently computes interval as 1000 / fps which will crash or produce nonsense for fps <= 0; validate the fps parameter at the start of animate_fluid_volume (or at least before creating the FuncAnimation) to ensure it's a positive number (e.g., if not isinstance(fps, (int,float)) or fps <= 0: raise ValueError("fps must be a positive number")), then proceed to compute interval = 1000.0 / fps and call FuncAnimation (referenced symbols: animate_fluid_volume, fps, FuncAnimation, interval).rocketpy/plots/motor_plots.py-524-580 (1)
524-580:⚠️ Potential issue | 🟡 MinorValidate
fpsbefore dividing by it.
1000 / fpsraises on0and produces an invalid interval for negative values; this public API should fail fast with a clearValueError.⏱️ Suggested guard
def animate_propellant_mass(self, filename=None, fps=30): """Animates the propellant mass of the motor as a function of time. @@ - # Extract time and mass data + if fps <= 0: + raise ValueError("fps must be greater than 0") + + # Extract time and mass data times = self.motor.propellant_mass.x_array values = self.motor.propellant_mass.y_array🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rocketpy/plots/motor_plots.py` around lines 524 - 580, In animate_propellant_mass validate the fps parameter before it's used (specifically before the FuncAnimation interval calculation 1000 / fps): check that fps is a positive non-zero number (int or float > 0) and raise a ValueError with a clear message if not; place this guard near the start of the function (after parameters are read) so invalid fps values fail fast and prevent division-by-zero or negative-intervals when building the FuncAnimation.tests/integration/motors/test_hybridmotor.py-21-55 (1)
21-55:⚠️ Potential issue | 🟡 MinorExercise the
HybridMotorAPI instead of mutatingmotor.soliddirectly.This bypasses the propagation path the test claims to cover, so it would still pass if
HybridMotor.only_radial_burnstopped syncing to the embeddedSolidMotor.🔧 Suggested test adjustment
@@ - motor.solid.only_radial_burn = True - assert motor.solid.only_radial_burn is True + motor.only_radial_burn = True + assert motor.only_radial_burn is True + assert motor.solid.only_radial_burn is True @@ - motor.solid.only_radial_burn = False + motor.only_radial_burn = False motor.solid.evaluate_geometry() - assert motor.solid.only_radial_burn is False + assert motor.only_radial_burn is False + assert motor.solid.only_radial_burn is False🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/motors/test_hybridmotor.py` around lines 21 - 55, The test mutates the embedded SolidMotor directly; instead exercise the HybridMotor API by setting motor.only_radial_burn = True/False and asserting motor.only_radial_burn, so propagation is validated; after toggling call motor.evaluate_geometry() (or the appropriate HybridMotor.update/evaluate method if named differently) to propagate changes to motor.solid, then compute burn areas and assert motor.solid.only_radial_burn matches and burn_area values as before.tests/integration/simulation/test_flight.py-762-810 (1)
762-810:⚠️ Potential issue | 🟡 MinorPatch
pyplot.showaround this newall_info()call.Line 810 is the only new
all_info()path in this file without the existing@patch("matplotlib.pyplot.show")guard. On interactive backends it can pop a window or block the test run.Suggested fix
+@patch("matplotlib.pyplot.show") def test_environment_methods_accessible_in_controller( - calisto_robust, example_plain_env -): + mock_show, calisto_robust, example_plain_env +): # pylint: disable=unused-argument🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/simulation/test_flight.py` around lines 762 - 810, The new call to flight.all_info() in test_environment_methods_accessible_in_controller can trigger a matplotlib window on interactive backends; wrap that invocation with a patch for matplotlib.pyplot.show (either add `@patch`("matplotlib.pyplot.show") to the test signature or use a context manager to patch show only around the flight.all_info() call) so the test does not block or pop a GUI; ensure the patched symbol is matplotlib.pyplot.show and keep the rest of the test unchanged.rocketpy/simulation/flight_comparator.py-77-83 (1)
77-83:⚠️ Potential issue | 🟡 MinorValidate
max_speedup front too.The new Flight-like check allows objects without
max_speed, butsummary()andcompare_key_events()read it unconditionally. That turns the promised constructor-time validation into a laterAttributeError.💡 Minimal fix
- required_attrs = ("t_final", "apogee", "apogee_time", "impact_velocity") + required_attrs = ( + "t_final", + "apogee", + "apogee_time", + "impact_velocity", + "max_speed", + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rocketpy/simulation/flight_comparator.py` around lines 77 - 83, The constructor's Flight-like validation misses the max_speed attribute so later calls in summary() and compare_key_events() can raise AttributeError; add "max_speed" to the required_attrs tuple in flight_comparator.py (the validation block that builds missing = [attr for attr in required_attrs if not hasattr(flight, attr)]) so the TypeError is raised up front and the error message includes max_speed in the listed required attributes.
Comment on lines
+2292
to
+2312
| if isinstance(input_data, (list, tuple)): | ||
| if all( | ||
| isinstance(item, (list, tuple)) and (len(item) == 2 or len(item) == 8) | ||
| for item in input_data | ||
| ): | ||
| try: | ||
| return self.__process_drag_input( | ||
| Function(list(input_data)), coeff_name | ||
| ) | ||
| except (TypeError, ValueError) as e: | ||
| raise ValueError( | ||
| f"Invalid list/tuple format for {coeff_name}. Expected " | ||
| "a list of [mach, coefficient] pairs or a list of " | ||
| "[alpha, beta, mach, reynolds, pitch_rate, yaw_rate, " | ||
| "roll_rate, coefficient] entries." | ||
| ) from e | ||
|
|
||
| raise TypeError( | ||
| f"Invalid input for {coeff_name}: must be int, float, CSV file path, " | ||
| "Function, or callable." | ||
| ) |
There was a problem hiding this comment.
np.ndarray drag tables regress here.
The constructor/docs still accept array drag inputs, but this branch only recognizes list/tuple. A documented np.ndarray now falls through to the final TypeError, so existing drag tables stop working.
💡 Minimal fix
- if isinstance(input_data, (list, tuple)):
+ if isinstance(input_data, (list, tuple, np.ndarray)):
if all(
- isinstance(item, (list, tuple)) and (len(item) == 2 or len(item) == 8)
+ isinstance(item, (list, tuple, np.ndarray))
+ and len(item) in (2, 8)
for item in input_data
):
try:
return self.__process_drag_input(
Function(list(input_data)), coeff_name
@@
raise TypeError(
- f"Invalid input for {coeff_name}: must be int, float, CSV file path, "
- "Function, or callable."
+ f"Invalid input for {coeff_name}: must be int, float, array-like, "
+ "CSV file path, Function, or callable."
)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rocketpy/rocket/rocket.py` around lines 2292 - 2312, The branch validating
list/tuple inputs for drag tables misses numpy arrays so np.ndarray inputs fall
through to the final TypeError; update the check around input_data in the
constructor (the isinstance(...) block that calls
self.__process_drag_input(Function(list(input_data)), coeff_name)) to also
accept numpy arrays (e.g., isinstance(input_data, (list, tuple, numpy.ndarray)))
or convert incoming np.ndarray to a list/tuple before calling
__process_drag_input/Function; if you add numpy to the isinstance check, import
numpy as np at the top (or use duck-typing like hasattr(input_data, "tolist")
and call input_data.tolist()) so existing array-based drag tables work again.
Comment on lines
1858
to
1861
| _, _, z, vx, vy, vz, e0, e1, e2, e3, omega1, omega2, omega3 = u | ||
| # Determine lift force and moment | ||
| omega1, omega2, omega3 = 0, 0, 0 | ||
| R1, R2, M1, M2, M3 = 0, 0, 0, 0, 0 |
There was a problem hiding this comment.
Angular rates are forcibly zeroed in u_dot.
Line 1860 overwrites omega1/omega2/omega3 with zeros right after state unpacking. This suppresses rotational coupling and can invalidate the intended dynamics.
Suggested fix
- omega1, omega2, omega3 = 0, 0, 0
R1, R2, M1, M2, M3 = 0, 0, 0, 0, 0📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| _, _, z, vx, vy, vz, e0, e1, e2, e3, omega1, omega2, omega3 = u | |
| # Determine lift force and moment | |
| omega1, omega2, omega3 = 0, 0, 0 | |
| R1, R2, M1, M2, M3 = 0, 0, 0, 0, 0 | |
| _, _, z, vx, vy, vz, e0, e1, e2, e3, omega1, omega2, omega3 = u | |
| # Determine lift force and moment | |
| R1, R2, M1, M2, M3 = 0, 0, 0, 0, 0 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rocketpy/simulation/flight.py` around lines 1858 - 1861, The code in u_dot
unpacks the state vector u into omega1/omega2/omega3 but then immediately
overwrites them with zeros, which suppresses rotational dynamics; remove the
line "omega1, omega2, omega3 = 0, 0, 0" so the function uses the unpacked
angular rates, and ensure downstream calculations that set R1, R2, M1, M2, M3
compute or initialize those variables based on the real omega values (or
explicitly document and compute any intended zeroing in a conditional), keeping
references to u, omega1, omega2, omega3 and the u_dot function intact.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…pytest standards (GitHub Copilot) (RocketPy-Team#937)
…ngelog (RocketPy-Team#940) * Initial plan * ENH: Add explicit timeouts to ThrustCurve API requests Co-authored-by: MateusStano <69485049+MateusStano@users.noreply.github.com> * DOC: Add timeout fix PR to changelog Co-authored-by: MateusStano <69485049+MateusStano@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: MateusStano <69485049+MateusStano@users.noreply.github.com>
…utes for raw user input and update changelog (RocketPy-Team#941) * Initial plan * ENH: Restore power_off/on_drag as Function, add _input attributes for raw user input Co-authored-by: MateusStano <69485049+MateusStano@users.noreply.github.com> * DOC: Add PR RocketPy-Team#941 compatibility fix to changelog Co-authored-by: MateusStano <69485049+MateusStano@users.noreply.github.com> * Update rocketpy/rocket/rocket.py Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com> * MNT: ruff pylint --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: MateusStano <69485049+MateusStano@users.noreply.github.com> Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com> Co-authored-by: MateusStano <mateusstano@usp.br>
…or.evaluate_geometry` (RocketPy-Team#944) * Initial plan * BUG: Fix incorrect Jacobian in only_radial_burn branch of evaluate_geometry Co-authored-by: MateusStano <69485049+MateusStano@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: MateusStano <69485049+MateusStano@users.noreply.github.com>
github-actions
bot
force-pushed
the
master
branch
from
8159eda to
697d668
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Creates scout_report.md containing three implementation briefs tailored for a first-time contributor based on selected issues from the upstream repository. The selected issues focus on utility, math/physics, and plotting, avoiding deep domain-specific logic.
PR created automatically by Jules for task 12924607668829467013 started by @khushalkottaru
Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests