Add classic acceleration mode#113
Conversation
|
@nexiumbiz-debug is attempting to deploy a commit to the gnarusg's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR introduces a new "Classic" acceleration mode supporting power-law sensitivity curves with configurable exponent. The feature spans kernel driver implementation, Rust parameter system, CLI/TUI interfaces, NixOS configuration, and comprehensive testing and documentation. ChangesClassic Acceleration Mode
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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)
module.nix (1)
45-45: 💤 Low valueUpdate comment to reflect Classic mode parameter.
Line 45 adds the
EXPONENTparameter under the# Linear mode parameterscomment (line 41), butEXPONENTis actually a Classic mode parameter. Consider adding a# Classic mode parameterscomment above line 45 to improve clarity.📝 Proposed documentation improvement
# Linear mode parameters ACCEL = cfg.parameters.acceleration; OFFSET = cfg.parameters.offset; OUTPUT_CAP = cfg.parameters.outputCap; + + # Classic mode parameters EXPONENT = cfg.parameters.exponent;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@module.nix` at line 45, The EXPONENT parameter is currently placed under the "# Linear mode parameters" heading but belongs to Classic mode; update the file by inserting a "# Classic mode parameters" comment immediately above the EXPONENT declaration (or move EXPONENT out from under the "# Linear mode parameters" block), ensuring the EXPONENT symbol is documented under the Classic mode section so headings accurately reflect parameter grouping.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@driver/accel/classic.h`:
- Around line 40-42: The current guard in the function that returns FIXEDPT_ONE
when input_speed <= args.offset or args.exponent <= 0 misses the explicit case
of input_speed == 0 which can lead to division by zero in classic_base_fn;
update the validation in the same function to explicitly check for input_speed
== 0 (or alternatively enforce/validate args.offset is non-negative) and return
FIXEDPT_ONE (or an appropriate safe value) when input_speed is zero to prevent
the division by zero in classic_base_fn.
In `@driver/accel/mode.h`:
- Around line 4-10: The enum declaration uses C++ typed-enum syntax which is
invalid in C; remove the ": unsigned char" part and declare a plain enum
accel_mode with the same enumerators (linear, natural, synchronous, no_accel,
classic) so the code compiles under C (gnu11); if a specific storage size is
required, replace usage sites to cast to/from uint8_t or add explicit storage
variables rather than relying on a typed enum.
---
Nitpick comments:
In `@module.nix`:
- Line 45: The EXPONENT parameter is currently placed under the "# Linear mode
parameters" heading but belongs to Classic mode; update the file by inserting a
"# Classic mode parameters" comment immediately above the EXPONENT declaration
(or move EXPONENT out from under the "# Linear mode parameters" block), ensuring
the EXPONENT symbol is documented under the Classic mode section so headings
accurately reflect parameter grouping.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6a67f19f-4bf7-4bf2-a5e3-12db5126d5af
📒 Files selected for processing (15)
README.mdREADME_NIXOS.mdcli/src/main.rscrates/core/src/context.rscrates/core/src/params.rscrates/core/src/persist.rscrates/core/src/sens_fns.rsdriver/accel.hdriver/accel/classic.hdriver/accel/mode.hdriver/accel_k.hdriver/params.hdriver/tests/accel.test.cmodule.nixtui/src/app.rs
|
Thanks for the review. I pushed commit 6066a1d to address the two actionable items here: the classic sensitivity path now explicitly returns early for zero input speed before the division path, and the driver mode enum is back to a plain C enum while the C-facing tag field stays an unsigned char to preserve the existing Rust/C layout. I also added the zero-speed regression assertion and moved EXPONENT under the classic-mode parameter block in the Nix module. I couldn't run the kernel driver build locally in this Windows environment, but git diff --check is clean. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
driver/tests/accel.test.c (1)
109-138: ⚡ Quick winConsider testing multiple speed values for robustness.
The test validates the core behaviors (exponent=2 equivalence, zero-speed edge case, exponent effect) but uses only a single speed value (4) for the exponent comparisons. Testing multiple speeds would strengthen confidence in the power-law curve.
🧪 Optional: Add multi-speed validation
static int test_classic_sensitivity(void) { struct linear_curve_args linear_args = (struct linear_curve_args){ .accel = fpt_rconst(0.3), .offset = fpt_fromint(1), .output_cap = 0, }; struct classic_curve_args classic_args = (struct classic_curve_args){ .accel = fpt_rconst(0.3), .offset = fpt_fromint(1), .output_cap = 0, .exponent = fpt_fromint(2), }; - fpt speed = fpt_fromint(4); - fpt linear_sens = __linear_sens_fun(speed, linear_args); - fpt classic_sens = __classic_sens_fun(speed, classic_args); - - assert(linear_sens == classic_sens); + // Test exponent=2 equivalence at multiple speeds + for (int s = 1; s <= 5; s++) { + fpt speed = fpt_fromint(s); + fpt linear_sens = __linear_sens_fun(speed, linear_args); + fpt classic_sens = __classic_sens_fun(speed, classic_args); + assert(linear_sens == classic_sens); + } classic_args.offset = -FIXEDPT_ONE; assert(__classic_sens_fun(0, classic_args) == FIXEDPT_ONE); classic_args.offset = fpt_fromint(1); classic_args.exponent = fpt_fromint(3); - fpt steeper_classic_sens = __classic_sens_fun(speed, classic_args); - - assert(steeper_classic_sens > classic_sens); + // Verify exponent effect holds across speeds + for (int s = 1; s <= 5; s++) { + fpt speed = fpt_fromint(s); + classic_args.exponent = fpt_fromint(2); + fpt baseline_sens = __classic_sens_fun(speed, classic_args); + classic_args.exponent = fpt_fromint(3); + fpt steeper_sens = __classic_sens_fun(speed, classic_args); + assert(steeper_sens > baseline_sens); + } return 0; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@driver/tests/accel.test.c` around lines 109 - 138, The test only checks sensitivity at speed=4; update test_classic_sensitivity to validate behavior across multiple speeds by iterating a small set of representative speeds (e.g., 0, 1, 2, 4, 8 or similar) and for each speed assert that __linear_sens_fun(speed, linear_args) == __classic_sens_fun(speed, classic_args) when exponent==2, and that increasing classic_args.exponent (e.g., from 2 to 3) yields a strictly larger __classic_sens_fun(speed, classic_args) for each nonzero speed; retain the separate zero-speed/offset edge-case check using __classic_sens_fun(0, classic_args) == FIXEDPT_ONE.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@driver/tests/accel.test.c`:
- Around line 109-138: The test only checks sensitivity at speed=4; update
test_classic_sensitivity to validate behavior across multiple speeds by
iterating a small set of representative speeds (e.g., 0, 1, 2, 4, 8 or similar)
and for each speed assert that __linear_sens_fun(speed, linear_args) ==
__classic_sens_fun(speed, classic_args) when exponent==2, and that increasing
classic_args.exponent (e.g., from 2 to 3) yields a strictly larger
__classic_sens_fun(speed, classic_args) for each nonzero speed; retain the
separate zero-speed/offset edge-case check using __classic_sens_fun(0,
classic_args) == FIXEDPT_ONE.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e0478bb8-a8e1-4ddc-9171-73398d21e404
📒 Files selected for processing (5)
driver/accel.hdriver/accel/classic.hdriver/accel/mode.hdriver/tests/accel.test.cmodule.nix
🚧 Files skipped from review as they are similar to previous changes (3)
- driver/accel/classic.h
- driver/accel.h
- module.nix
|
Small follow-up in 43774ae: broadened the classic sensitivity test across multiple representative speeds and kept the zero-speed edge-case assertion separate. Local validation: git diff --check is clean. I still cannot run make test from this Windows shell because make/WSL/C toolchain are not installed here; the PR's CodeRabbit check is passing on the updated head, with Vercel still awaiting team authorization. |
|
Hey, just checking in on this one. CodeRabbit is passing now and the only visible check still failing is Vercel asking for team authorization, so I think the implementation is ready for a maintainer look whenever you have a moment. |
2 participants
Summary
classicacceleration mode with a configurableEXPONENTparameter.classicafterno_accel; exponent2matches the existing linear formula path.Fixes #111.
Verification
git diff --check.cargo fmt --all,cargo test --all, andmake test; this Windows Codex environment has nocargo,gcc,make, or installed WSL distro available.Note
This PR was prepared with AI assistance from the
nexiumbiz-debugaccount. If this solves the requested feature and sponsorship/donation coordination is needed, please handle that privately with the account owner rather than in the public issue thread.Summary by CodeRabbit