[bernardogv]

Spec Driven Development

This PR:

• fixes something and does not need a WG/RFC.
• is within the scope of WG:
• has approved RFC #

Summary

Change	Router::load_model: skip the nuclear evict-and-retry when loaded_servers_ is empty (the failed server is never in it)
Before	a model that doesn't fit is launched twice with nothing evicted between: two kernel OOM kills (Ryzen AI Max+ 395, Ubuntu 26.04)
Unchanged	with another model loaded: evict all + one retry, as before
Trade-off	no free retry for transient startup failures when nothing else is loaded
Note	the comment above this branch cites "spec: Error Handling section"; no such section in docs/
Tests	test_failed_load_with_nothing_loaded_is_not_retried, test_failed_load_with_another_model_loaded_is_retried (mock llama-server gains fail_startup / fail_startup_arg)

Scope

• This PR addresses one clear issue or change.
• I reviewed the full diff myself before submitting.
• I removed unrelated local changes.
• I kept refactoring separate unless it is required for this change.

Testing

• The code change has been locally tested.

Testing details:

Build	test/test_gpu_hang_retry.py
fix	5/5 OK
router.cpp reverted to main	AssertionError: 2 != 1 : Expected one backend launch, got 2 (retried)
guard forced to always skip	AssertionError: 2 != 3 : Expected first launch + failed load + one retry, got 2

# ubuntu:26.04 container
cmake --preset default -DBUILD_TESTING=ON
cmake --build --preset default --target lemond lemonade
python test/test_gpu_hang_retry.py --cli-binary build/lemonade

Documentation

• Documentation is not affected by this change.
• Documentation is affected and has been updated.

Breaking Changes

• This PR introduces breaking changes.
• This PR does not introduce breaking changes.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 343648bc-1c39-46c1-87e2-4fb64d199ab9

📥 Commits

Reviewing files that changed from the base of the PR and between 80891db and ccf53d7.

📒 Files selected for processing: 2

• src/cpp/server/router.cpp
• test/test_gpu_hang_retry.py

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

📝 Summary

Summary by CodeRabbit

• Bug Fixes
  • Failed model loads now return the original error without attempting an eviction-and-retry when no other models are loaded, avoiding an unnecessary retry and surfacing startup failures directly.
  • When another model is loaded, existing recovery behavior remains: that model may be evicted and the failed load retried, with the result reflecting the retry outcome.

When a model load fails for a reason other than file not found, the router now skips eviction and retry if no models are loaded. Tests cover this behavior and confirm that the existing eviction-and-retry path remains when another model is loaded.

Changes: Model load failure retry handling

Layer / File(s)	Summary
Retry behavior and regression tests src/cpp/server/router.cpp, test/test_gpu_hang_retry.py	The router throws the original load error without retry when no models are loaded. The mock backend can fail at startup based on test state or a command-line argument. Tests check the single-launch case and the retry after eviction when another model is loaded.

Suggested reviewers: jeremyfowers, abn

Priority: ➖ Normal

Change: Bug fix

Merge Risk: ⚪ Minimal · up to ccf53

This change stops a failed model load from launching the backend a second time when there is nothing to evict. That avoids repeated out-of-memory kills. Behavior when other models are loaded is unchanged. No merge-blocking risk remains.

Pre-merge checks: 6

Check name	Status	Explanation
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Ai Generated Pr		The pull request description does not contain the canary word “sweet.” No AI-canary failure is present.
Inconsistent Pr Body		The PR body matches the reviewed code. Router::load_model now throws the original non-file-not-found error when loaded_servers_ is empty, and it still evicts all models and retries when the list i…
Check Spec And Rfc		No linked spec or RFC document appears in the PR body. The RFC checklist is unchecked and the approved RFC field is blank, so the consistency check is not applicable.
Description check		The pull request description includes all required sections. It states the scope decision, summarizes the change, documents testing commands and results, addresses documentation impact, and identifies…

Full details: Inconsistent Pr Body

Explanation

The PR body matches the reviewed code. Router::load_model now throws the original non-file-not-found error when loaded_servers_ is empty, and it still evicts all models and retries when the list is non-empty. The two named tests exist and cover one launch with no loaded model and one failed launch plus one retry with another model loaded. The file also contains the cited Error Handling section comment, while no matching specification section appears under docs/.

Full details: Description check

Explanation

The pull request description includes all required sections. It states the scope decision, summarizes the change, documents testing commands and results, addresses documentation impact, and identifies breaking changes. The description is specific and aligns with the reported implementation and tests.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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