Skip to content

fix: pass project_root instead of checkpoint dir to initialize_service#637

Merged
ChuxiJ merged 1 commit intomainfrom
fix_checkpoint_init_dir
Feb 19, 2026
Merged

fix: pass project_root instead of checkpoint dir to initialize_service#637
ChuxiJ merged 1 commit intomainfrom
fix_checkpoint_init_dir

Conversation

@ChuxiJ
Copy link
Contributor

@ChuxiJ ChuxiJ commented Feb 19, 2026

init_service_wrapper was passing the checkpoint dropdown value (the full checkpoints directory path, e.g. '/checkpoints') as the project_root argument to initialize_service(). This caused initialize_service to append 'checkpoints' again, resulting in '/checkpoints/checkpoints' — triggering unnecessary model re-downloads into the nested directory.

Fix: derive project_root from file (consistent with the LLM init path that was already correct) instead of using the checkpoint dropdown value.

Adds unit tests verifying the project_root is never the checkpoints dir.

Summary by CodeRabbit

  • Refactor

    • Improved initialization path computation to eliminate redundant calculations and ensure consistency.
  • Tests

    • Added unit tests to validate proper path handling during service initialization.

init_service_wrapper was passing the checkpoint dropdown value (the full
checkpoints directory path, e.g. '<project>/checkpoints') as the
project_root argument to initialize_service(). This caused
initialize_service to append 'checkpoints' again, resulting in
'<project>/checkpoints/checkpoints' — triggering unnecessary model
re-downloads into the nested directory.

Fix: derive project_root from __file__ (consistent with the LLM init
path that was already correct) instead of using the checkpoint dropdown
value.

Adds unit tests verifying the project_root is never the checkpoints dir.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 19, 2026

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

This PR refactors path derivation in the service initialization module to compute project_root once from the file path and reuse it, eliminating duplicate "checkpoints" directory appending. A comprehensive test suite validates the corrected behavior.

Changes

Cohort / File(s) Summary
Path Derivation Refactor
acestep/ui/gradio/events/generation/service_init.py
Moved project_root computation to a single location prior to initialize_service call, removing redundant recomputation inside the init_llm block and fixing double "checkpoints" directory appending.
Checkpoint Path Tests
acestep/ui/gradio/events/generation/service_init_test.py
Added new test module with InitServiceWrapperPathTests class containing two test methods validating that project_root is derived from module path (not checkpoint dropdown) and that checkpoint directory nesting is not duplicated.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Once was a path, doubled and bent,
"Checkpoints" nested—oh what a spent!
Now single derivation, clean and pure,
Tests guard the logic, steady and sure.
Files find their home, no more confusion,
A hopper's delight, bug-free fusion! 🥕

✨ Finishing Touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix_checkpoint_init_dir

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

@ChuxiJ ChuxiJ merged commit 47315e8 into main Feb 19, 2026
1 of 2 checks passed
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

Comments