Skip to content

[config] fix: constrain busy threshold range - #79

Merged
Wangmerlyn merged 4 commits into
mainfrom
codex/fix-busy-threshold-range
Jun 27, 2026
Merged

[config] fix: constrain busy threshold range#79
Wangmerlyn merged 4 commits into
mainfrom
codex/fix-busy-threshold-range

Conversation

@Wangmerlyn

@Wangmerlyn Wangmerlyn commented Jun 27, 2026

Copy link
Copy Markdown
Owner

Summary

  • constrain busy_threshold to -1 or inclusive 0..100 in the central validator
  • validate CLI start/blocking threshold values before service startup or controller construction
  • document the percentage range in AGENTS.md, README, CLI/MCP/Python guides, architecture notes, and the branch plan

Verification

  • PYTHONPATH=$PWD/src pytest tests/utilities/test_session_config.py tests/test_cli_thresholds.py tests/test_cli_service_commands.py tests/mcp/test_server.py tests/cuda_controller/test_throttle.py tests/global_controller/test_contract.py -q -> 70 passed, 1 skipped
  • PYTHONPATH=$PWD/src pytest tests -q -> 107 passed, 12 skipped
  • PYTHONPATH=$PWD/src mkdocs build -> built successfully with existing Material/MkDocs and unnav'd docs notices
  • pre-commit run --all-files -> passed
  • git diff --check -> passed

Local worker notes

  • Lovelace performed the TDD source/test slice and confirmed 101 failed before the central validator fix, then passed after it.
  • I added CLI preflight validation so invalid thresholds do not auto-start the service before rejection.

Summary by CodeRabbit

  • New Features

    • Tightened busy_threshold handling across the app to accept only -1 or values from 0 to 100.
    • Clarified how backoff behaves when telemetry is unavailable, including when the controller sleeps versus runs continuously.
  • Bug Fixes

    • Invalid threshold values now fail early in the CLI and API flows with clearer error messages.
    • Added coverage for boundary cases like 100 being allowed and 101 being rejected.

@coderabbitai

coderabbitai Bot commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@Wangmerlyn, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 32 minutes and 40 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7505ade2-d2fd-418d-9a81-c1ee3a73c0f9

📥 Commits

Reviewing files that changed from the base of the PR and between 7aebb40 and e44f881.

📒 Files selected for processing (9)
  • docs/plans/busy-threshold-range.md
  • src/keep_gpu/cli.py
  • src/keep_gpu/mcp/static/assets/dashboard.js
  • tests/mcp/test_http_api.py
  • tests/test_cli_thresholds.py
  • tests/utilities/test_session_config.py
  • web/dashboard/src/App.jsx
  • web/dashboard/src/lib/session.js
  • web/dashboard/src/lib/session.test.js
📝 Walkthrough

Walkthrough

validate_busy_threshold is updated to accept only -1 or integers in 0..100, replacing the old >= -1 check. A new _validate_cli_busy_threshold helper wraps this for Typer CLI errors and is applied in both the blocking and start command paths. Tests and all documentation are updated to reflect the new constraint.

busy_threshold range enforcement

Layer / File(s) Summary
validate_busy_threshold logic update
src/keep_gpu/utilities/session_config.py, tests/utilities/test_session_config.py, tests/cuda_controller/test_throttle.py, tests/global_controller/test_contract.py
validate_busy_threshold now rejects values outside -1 or 0..100; existing tests updated to match new error message and new boundary tests added for 100 (accepted) and 101 (rejected).
CLI validation wiring
src/keep_gpu/cli.py, tests/test_cli_service_commands.py, tests/test_cli_thresholds.py
_validate_cli_busy_threshold converts ValueError to typer.BadParameter and is applied in both the blocking and start command paths; new CLI tests confirm --busy-threshold 101 exits with code 1.
MCP rejection test
tests/mcp/test_server.py
New JSON-RPC test asserts start_keep rejects busy_threshold=101 with the validation error and leaves no active jobs.
Documentation updates
docs/plans/busy-threshold-range.md, AGENTS.md, README.md, docs/concepts/architecture.md, docs/guides/cli.md, docs/guides/mcp.md, docs/guides/python.md, docs/reference/cli.md
All docs updated to state valid busy_threshold values (-1 or 0..100) and clarify backoff/sleep behavior when telemetry is unavailable.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

A rabbit checks the threshold gate,
No 101 shall pass — too late!
From -1 to a hundred true,
The GPU sleeps when util's askew.
🐇 Bounds enforced, the cluster's great!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.53% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately summarizes the main change: constraining the busy threshold range.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-busy-threshold-range

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request restricts the busy_threshold configuration to accept only -1 or integers between 0 and 100 (inclusive) to ensure eco-safe utilization backoff. It updates the centralized validator, integrates it into the CLI, updates extensive documentation, and adds corresponding test coverage. Feedback suggests using a more specific exception class (typer.BadParameter instead of Exception) in the CLI threshold tests to avoid overly broad assertions.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread tests/test_cli_thresholds.py

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@tests/test_cli_thresholds.py`:
- Around line 38-40: The threshold CLI test is asserting on a слишком broad
exception type, so tighten it to match the contract of
_validate_cli_busy_threshold(). Update the pytest.raises call in the busy
threshold test to expect typer.BadParameter instead of Exception, and keep the
existing error message match so the test verifies the intended validation path
in test_cli_thresholds.py.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 349f92e1-e164-4c14-89bf-0b8ea7c1bac5

📥 Commits

Reviewing files that changed from the base of the PR and between 4e78819 and 7aebb40.

📒 Files selected for processing (16)
  • AGENTS.md
  • README.md
  • docs/concepts/architecture.md
  • docs/guides/cli.md
  • docs/guides/mcp.md
  • docs/guides/python.md
  • docs/plans/busy-threshold-range.md
  • docs/reference/cli.md
  • src/keep_gpu/cli.py
  • src/keep_gpu/utilities/session_config.py
  • tests/cuda_controller/test_throttle.py
  • tests/global_controller/test_contract.py
  • tests/mcp/test_server.py
  • tests/test_cli_service_commands.py
  • tests/test_cli_thresholds.py
  • tests/utilities/test_session_config.py

Comment thread tests/test_cli_thresholds.py
@Wangmerlyn
Wangmerlyn merged commit 8b81e98 into main Jun 27, 2026
5 checks passed
@Wangmerlyn
Wangmerlyn deleted the codex/fix-busy-threshold-range branch June 27, 2026 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant