Repository navigation
Refactor velocity calculation to growth-based model for intuitive DX - #24
Conversation
Co-authored-by: SomeRandmGuyy <127457986+SomeRandmGuyy@users.noreply.github.com>
Co-authored-by: SomeRandmGuyy <127457986+SomeRandmGuyy@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the Terminal Velocity calculation model from an inverse time-based approach to a growth-based model where velocity increases with uptime. The new model is more intuitive (higher values = better performance) and adds configurable parameters for acceleration and maximum velocity caps.
- Replaced inverse velocity calculation (
1.0 / uptime) with a growth-based formula (BASE_VELOCITY + uptime * acceleration) - Added configurable acceleration factor and maximum velocity parameters with validation
- Added comprehensive documentation explaining the velocity model, configuration, and usage examples
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/fsl_continuum/continuum/terminal_velocity.py | Rewrote velocity calculation logic, added configuration parameters, new setter/getter methods, and enhanced documentation |
| docs/0003-architecture/terminal-velocity-quickref.md | New quick reference guide with usage examples and configuration patterns |
| docs/0003-architecture/terminal-velocity-model.md | New comprehensive documentation covering the velocity model, API reference, and migration guide |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| """Calculate terminal velocity. | ||
|
|
||
| Uses a growth-based model where velocity increases with uptime. | ||
| Formula: velocity = BASE + (uptime * acceleration) * flow_multiplier |
There was a problem hiding this comment.
The formula in the comment is incorrect. According to the implementation on lines 72-76, the flow multiplier is applied to the entire velocity after acceleration, not just to (uptime * acceleration). The correct formula should be: velocity = (BASE + uptime * acceleration) * flow_multiplier (when in flow state).
| Formula: velocity = BASE + (uptime * acceleration) * flow_multiplier | |
| Formula: velocity = (BASE + uptime * acceleration) * flow_multiplier (when in flow state) |
There was a problem hiding this comment.
Fixed in commit 6a7955b. The formula documentation now correctly shows that the flow multiplier is applied to the entire velocity: velocity = (BASE + uptime * acceleration) * flow_multiplier (when in flow state)
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| """Calculate terminal velocity. | ||
|
|
||
| Uses a growth-based model where velocity increases with uptime. | ||
| Formula: velocity = BASE + (uptime * acceleration) * flow_multiplier |
There was a problem hiding this comment.
syntax: Formula in docstring doesn't match implementation. Docstring says (uptime * acceleration) * flow_multiplier but code applies flow multiplier AFTER calculating base velocity, not during.
| Formula: velocity = BASE + (uptime * acceleration) * flow_multiplier | |
| Formula: velocity = (BASE + uptime * acceleration) * flow_multiplier (if in flow) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/fsl_continuum/continuum/terminal_velocity.py
Line: 63:63
Comment:
**syntax:** Formula in docstring doesn't match implementation. Docstring says `(uptime * acceleration) * flow_multiplier` but code applies flow multiplier AFTER calculating base velocity, not during.
```suggestion
Formula: velocity = (BASE + uptime * acceleration) * flow_multiplier (if in flow)
```
How can I resolve this? If you propose a fix, please make it concise.
|
@copilot apply changes based on the comments in this thread |
… application Co-authored-by: SomeRandmGuyy <127457986+SomeRandmGuyy@users.noreply.github.com>
Applied the fix from the review comment. The formula documentation has been corrected to accurately reflect that the flow state multiplier is applied to the entire velocity calculation, not just the acceleration component. See commit 6a7955b. |
Summary
Successfully reworked the terminal velocity calculation to address DX concerns:
Before: Inverse model where velocity decreased over time (counterintuitive)
After: Growth-based model where velocity increases over time (intuitive)
Key improvements:
The system now speeds up with higher values as requested, making it intuitive from a DX perspective while maintaining sound business logic.
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.
Greptile Overview
Updated On: 2025-11-04 07:04:30 UTC
Greptile Summary
Refactored velocity calculation from inverse model to growth-based model, making DX more intuitive (higher values = better performance).
Key Changes:
1.0 / uptimewithBASE_VELOCITY + (uptime × acceleration)growth modelaccelerationandmax_velocityconstructor parametersset_acceleration(),set_max_velocity(),get_config()get_status()to include configuration detailsstart_timetruthiness checkDocumentation:
Issues Found:
Confidence Score: 4/5
Important Files Changed
File Analysis
Sequence Diagram
sequenceDiagram participant Client participant TerminalVelocity participant Timer as time.time() Client->>TerminalVelocity: __init__(acceleration, max_velocity) activate TerminalVelocity TerminalVelocity->>Timer: time.time() Timer-->>TerminalVelocity: start_time TerminalVelocity->>TerminalVelocity: Set config (acceleration, max_velocity) TerminalVelocity->>TerminalVelocity: flow_state = False deactivate TerminalVelocity Client->>TerminalVelocity: enter_flow_state() activate TerminalVelocity TerminalVelocity->>TerminalVelocity: flow_state = True TerminalVelocity->>Timer: time.time() Timer-->>TerminalVelocity: flow_state_start deactivate TerminalVelocity Client->>TerminalVelocity: get_velocity() activate TerminalVelocity TerminalVelocity->>Timer: time.time() Timer-->>TerminalVelocity: current_time TerminalVelocity->>TerminalVelocity: uptime = current_time - start_time TerminalVelocity->>TerminalVelocity: velocity = BASE_VELOCITY + (uptime × acceleration) alt flow_state is True TerminalVelocity->>TerminalVelocity: velocity × 1.5 end TerminalVelocity->>TerminalVelocity: velocity = min(velocity, max_velocity) TerminalVelocity-->>Client: velocity (higher = better) deactivate TerminalVelocity Client->>TerminalVelocity: set_acceleration(new_value) activate TerminalVelocity TerminalVelocity->>TerminalVelocity: Validate new_value >= 0 TerminalVelocity->>TerminalVelocity: acceleration = new_value deactivate TerminalVelocity Client->>TerminalVelocity: exit_flow_state() activate TerminalVelocity TerminalVelocity->>TerminalVelocity: flow_state = False TerminalVelocity->>Timer: time.time() Timer-->>TerminalVelocity: current_time TerminalVelocity->>TerminalVelocity: Calculate flow duration TerminalVelocity->>TerminalVelocity: Store last_flow_duration metric deactivate TerminalVelocity