Conversation
…aths_follow_executable
Added license information and updated project badges.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughCelune 4.1.0 adds compiled launcher distribution with in-app updates, a Gradio-based browser WebUI, a shared playback mixer for audio ducking, internal state refactoring via dataclasses, and Qwen3-VL persona model support. The launcher and build scripts enable automatic binary updates. The playback mixer coordinates speech and SFX with per-source gain and glow synchronization. The WebUI mirrors runtime state and accepts slash commands. Core classes reorganize mutable state into grouped containers for clarity. ChangesCore Launcher and Build Infrastructure
Core State Refactoring
Audio Playback and Mixer Refactor
WebUI Integration
Backend and Model Updates
Testing and Documentation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly Related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (7)
celune/ui/app.py (1)
107-114: ⚡ Quick winSimplify the default value assignment.
Lines 107-109 wrap
Nonein parentheses across multiple lines, which is unnecessarily verbose.♻️ Simplify formatting
- runtime_redirect_original_handlers: Optional[dict[str, list[logging.Handler]]] = ( - None - ) + runtime_redirect_original_handlers: Optional[dict[str, list[logging.Handler]]] = None🤖 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 `@celune/ui/app.py` around lines 107 - 114, The three attribute defaults are written with unnecessary multi-line parentheses; simplify by assigning None directly to runtime_redirect_original_handlers and runtime_redirect_original_propagate and use single-line declarations for runtime_redirect_original_handlers, runtime_redirect_original_propagate, and warnings_capture_enabled in the class where they are defined (referencing runtime_redirect_original_handlers, runtime_redirect_original_propagate, and warnings_capture_enabled) so each default is a direct simple literal (e.g., runtime_redirect_original_handlers: Optional[dict[str, list[logging.Handler]]] = None) instead of the wrapped multi-line form.celune/entrypoint.py (1)
285-291: 💤 Low valueDocument why compiled execution uses venv Python for doctor repairs.
The
_doctor_subprocess_python()function returns_doctor_venv_python()whenrunning_compiled()instead of the current interpreter. This design choice should be briefly commented to explain that the compiled binary cannot directly invoke setup.py, so doctor repairs delegate to the venv interpreter.🤖 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 `@celune/entrypoint.py` around lines 285 - 291, Add a brief inline comment to _doctor_subprocess_python explaining that when running_compiled() is true the compiled binary cannot directly invoke setup.py or perform interpreter-based repairs, so doctor fixes must be executed via the virtualenv interpreter returned by _doctor_venv_python(); otherwise use _doctor_running_python() for normal (non-compiled) runs. This clarifies the rationale behind choosing _doctor_venv_python over the current interpreter.celune/extensions/manager.py (1)
24-24: 💤 Low valueConsider using
typing.Typefor consistency with classic typing syntax guideline.The built-in
type[CeluneExtension](PEP 585) is valid for Python 3.12+, but the coding guideline prefers classic typing syntax. Consider:from typing import Type def register(self, extension_cls: Type[CeluneExtension]) -> CeluneExtension:This maintains consistency with the project's stated preference for classic typing module usage.
As per coding guidelines: prefer classic typing syntax.
🤖 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 `@celune/extensions/manager.py` at line 24, Replace the PEP 585-style annotation in the register method with the classic typing.Type usage: update the register signature in class Manager (register(self, extension_cls: type[CeluneExtension]) -> CeluneExtension) to use typing.Type for extension_cls and add the corresponding import from typing; ensure CeluneExtension remains the return type and the new Type[CeluneExtension] annotation references the same CeluneExtension symbol.Source: Coding guidelines
.github/workflows/ci.yml (2)
60-99: 💤 Low valueConsider pinning GitHub Actions to commit hashes for supply-chain security.
Static analysis flagged unpinned action references at lines 74 and 96. While version tags (
@v5,@v7) provide convenience, pinning to commit hashes prevents unexpected behavior from tag reassignments.Example:
- uses: astral-sh/setup-uv@bbf0a5ef00a28e8f6e8e6d245bd7b381b4cf1b68 # v7Note: This may be an intentional trade-off between security and maintainability.
🤖 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 @.github/workflows/ci.yml around lines 60 - 99, The workflow uses unpinned action tags (actions/checkout@v5, astral-sh/setup-uv@v7, actions/upload-artifact@v6) which can be maliciously reassigned; update each uses: reference to pin them to their specific commit SHA instead of the tag (replace `@v5/`@v7/@v6 with the corresponding full commit hash for each action) and verify the chosen SHAs match the intended tag versions before committing.Source: Linters/SAST tools
101-131: 💤 Low valueConsider pinning GitHub Actions to commit hashes for supply-chain security.
Static analysis flagged unpinned action references at lines 110 and 128. Additionally, line 106-107 has a credential persistence warning—consider adding
persist-credentials: falseto the checkout action if the workflow doesn't need to push commits.Example:
- name: Checkout repository uses: actions/checkout@<hash> # v5 with: persist-credentials: falseNote: These may be intentional trade-offs for this project.
🤖 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 @.github/workflows/ci.yml around lines 101 - 131, Update the workflow to pin third-party actions to immutable commit SHAs instead of tags for the steps named "Checkout repository" (uses: actions/checkout@v5), "Install uv" (uses: astral-sh/setup-uv@v7) and "Upload Windows launcher artifacts" (uses: actions/upload-artifact@v6) by replacing the tag with the corresponding commit hash; also add the with: persist-credentials: false option to the "Checkout repository" step to avoid leaking credentials when not needed. Ensure you reference and update the exact steps by their step names ("Checkout repository", "Install uv", "Upload Windows launcher artifacts") so the SHA pins and the persist-credentials setting are applied in the correct places.Source: Linters/SAST tools
celune/colors.py (1)
17-17: ⚡ Quick winUse classic type alias syntax instead of PEP 695
typestatement.The coding guideline prefers classic typing syntax. The
type RGB = ...statement is PEP 695 syntax (Python 3.12+). Consider using the classic type alias pattern:RGB = tuple[int, int, int]or with explicit annotation:
from typing import TypeAlias RGB: TypeAlias = tuple[int, int, int]As per coding guidelines: prefer classic typing syntax over newer PEP syntax features.
🤖 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 `@celune/colors.py` at line 17, Replace the PEP 695 "type" statement that defines the RGB alias with the classic type-alias pattern: change the RGB declaration to a simple assignment-style alias to a 3-tuple of ints, or alternatively annotate RGB using typing.TypeAlias, so the name RGB resolves as the intended tuple[int, int, int] type; update the RGB declaration in colors.py accordingly.Source: Coding guidelines
celune/api.py (1)
556-732: 💤 Low valueDynamic attribute injection for callback tracking.
Line 732 uses
setattr(celune, "_webui_callbacks_wrapped", True)to track whether callbacks have been wrapped. While functional, this injects a private attribute onto theCeluneinstance.Consider adding a typed attribute to the
Celuneclass if this pattern persists, or documenting the convention. For now, the approach works and prevents double-wrapping.🤖 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 `@celune/api.py` around lines 556 - 732, The current callback-wrapping uses dynamic attribute injection via setattr(celune, "_webui_callbacks_wrapped", True) in _wrap_celune_callbacks which mutates the Celune instance at runtime; instead add a typed attribute _webui_callbacks_wrapped: bool = False to the Celune class definition and update _wrap_celune_callbacks to read and set that attribute directly (e.g., check getattr/celune._webui_callbacks_wrapped and assign celune._webui_callbacks_wrapped = True) so the flag is part of the class surface rather than injected dynamically.
🤖 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 `@celune/pipeline.py`:
- Around line 2356-2361: When _ensure_playback_stream(engine, BASE_SR) returns
False you must call release_pipeline(engine) before clearing mixer state and
breaking so engine.locked is cleared; update the failure branch that currently
clears
source_buffers/source_done/_playback_source_statuses(engine)/_playback_source_meta(engine)
to first call release_pipeline(engine) (or the appropriate release_pipeline call
used elsewhere) and then clear state and break, ensuring subsequent say()/play()
calls are not rejected due to engine.locked remaining True.
- Around line 833-858: The subprocess call in _download_youtube_sfx uses
subprocess.run without a timeout which can hang; add a reasonable timeout
argument (e.g., timeout=<seconds>) to the subprocess.run call that invokes
yt_dlp_module and wrap the call in a try/except that catches
subprocess.TimeoutExpired and handles it the same way as the current non-zero
return/error paths (log the timeout, set completed/return appropriate failure
state, and clean up any partial output) so behavior matches existing error
handling for _download_youtube_sfx.
In `@GALLERY.md`:
- Around line 17-19: The "Change voice" section heading is plain text and needs
to be a Markdown heading; update the line containing "Change voice" to a proper
heading (e.g., prefix with a Markdown header token like "### " or the same
heading level used by other gallery sections) so the section renders
consistently with the other headings and leaves the existing image/link block
intact.
In `@scripts/build_nuitka.ps1`:
- Line 76: After running the Nuitka command invocation (& uv `@arguments`) check
the PowerShell exit code stored in $LASTEXITCODE and abort the script if it is
non-zero; specifically, immediately after the & uv `@arguments` call inspect
$LASTEXITCODE and call exit or throw an error to prevent continuing to the C
launcher compilation step (the subsequent launcher build), so the script only
proceeds to compile the launcher when the Nuitka step succeeded.
- Around line 12-13: The environment variables $env:CL and $env:_CL_ are set
globally before the Nuitka invocation (& uv `@arguments`) so they affect both
Nuitka's MSVC invocation and the later manual compile/link command stored in
$compileCmd; scope them only for the launcher build by setting and restoring
them immediately around the launcher invocation (or explicitly pass equivalent
flags to Nuitka) instead of leaving them set for the whole script—wrap the
temporary assignments in a local/try/finally or save original values, set
$env:CL and $env:_CL_ just before running cmd /c $compileCmd (or before & uv
`@arguments` if the intent is for Nuitka only), then restore the originals after
that command completes so $compileCmd runs with the original environment.
In `@scripts/build_nuitka.sh`:
- Around line 99-143: The manifest currently hardcodes "Celune-linux-x64"; pass
the detected appimage_arch into the Python block and use it when building
manifest["artifact"]. Concretely, set CELUNE_ARCH="$appimage_arch" when invoking
the Python runner (or export it beforehand) where you call appimage_arch and run
uv run python -, and inside that Python snippet read os.environ["CELUNE_ARCH"]
and set manifest["artifact"] to "Celune-linux-" + that value (use the existing
manifest variable in the Python block). This ensures the artifact string
reflects the appimage_arch value determined earlier.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 60-99: The workflow uses unpinned action tags
(actions/checkout@v5, astral-sh/setup-uv@v7, actions/upload-artifact@v6) which
can be maliciously reassigned; update each uses: reference to pin them to their
specific commit SHA instead of the tag (replace `@v5/`@v7/@v6 with the
corresponding full commit hash for each action) and verify the chosen SHAs match
the intended tag versions before committing.
- Around line 101-131: Update the workflow to pin third-party actions to
immutable commit SHAs instead of tags for the steps named "Checkout repository"
(uses: actions/checkout@v5), "Install uv" (uses: astral-sh/setup-uv@v7) and
"Upload Windows launcher artifacts" (uses: actions/upload-artifact@v6) by
replacing the tag with the corresponding commit hash; also add the with:
persist-credentials: false option to the "Checkout repository" step to avoid
leaking credentials when not needed. Ensure you reference and update the exact
steps by their step names ("Checkout repository", "Install uv", "Upload Windows
launcher artifacts") so the SHA pins and the persist-credentials setting are
applied in the correct places.
In `@celune/api.py`:
- Around line 556-732: The current callback-wrapping uses dynamic attribute
injection via setattr(celune, "_webui_callbacks_wrapped", True) in
_wrap_celune_callbacks which mutates the Celune instance at runtime; instead add
a typed attribute _webui_callbacks_wrapped: bool = False to the Celune class
definition and update _wrap_celune_callbacks to read and set that attribute
directly (e.g., check getattr/celune._webui_callbacks_wrapped and assign
celune._webui_callbacks_wrapped = True) so the flag is part of the class surface
rather than injected dynamically.
In `@celune/colors.py`:
- Line 17: Replace the PEP 695 "type" statement that defines the RGB alias with
the classic type-alias pattern: change the RGB declaration to a simple
assignment-style alias to a 3-tuple of ints, or alternatively annotate RGB using
typing.TypeAlias, so the name RGB resolves as the intended tuple[int, int, int]
type; update the RGB declaration in colors.py accordingly.
In `@celune/entrypoint.py`:
- Around line 285-291: Add a brief inline comment to _doctor_subprocess_python
explaining that when running_compiled() is true the compiled binary cannot
directly invoke setup.py or perform interpreter-based repairs, so doctor fixes
must be executed via the virtualenv interpreter returned by
_doctor_venv_python(); otherwise use _doctor_running_python() for normal
(non-compiled) runs. This clarifies the rationale behind choosing
_doctor_venv_python over the current interpreter.
In `@celune/extensions/manager.py`:
- Line 24: Replace the PEP 585-style annotation in the register method with the
classic typing.Type usage: update the register signature in class Manager
(register(self, extension_cls: type[CeluneExtension]) -> CeluneExtension) to use
typing.Type for extension_cls and add the corresponding import from typing;
ensure CeluneExtension remains the return type and the new Type[CeluneExtension]
annotation references the same CeluneExtension symbol.
In `@celune/ui/app.py`:
- Around line 107-114: The three attribute defaults are written with unnecessary
multi-line parentheses; simplify by assigning None directly to
runtime_redirect_original_handlers and runtime_redirect_original_propagate and
use single-line declarations for runtime_redirect_original_handlers,
runtime_redirect_original_propagate, and warnings_capture_enabled in the class
where they are defined (referencing runtime_redirect_original_handlers,
runtime_redirect_original_propagate, and warnings_capture_enabled) so each
default is a direct simple literal (e.g., runtime_redirect_original_handlers:
Optional[dict[str, list[logging.Handler]]] = None) instead of the wrapped
multi-line form.
🪄 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: 8c426611-2f3e-4cf7-96c7-3a1c593cd1fe
⛔ Files ignored due to path filters (12)
celune.exeis excluded by!**/*.exeresources/branding/celune.svgis excluded by!**/*.svgresources/branding/celune_88x31_206.pngis excluded by!**/*.pngresources/branding/celune_88x31_206_light.pngis excluded by!**/*.pngresources/branding/celune_88x31_basic.pngis excluded by!**/*.pngresources/branding/celune_88x31_basic_light.pngis excluded by!**/*.pngresources/branding/celune_dark.svgis excluded by!**/*.svgresources/branding/celune_wordmark.pngis excluded by!**/*.pngresources/branding/celune_wordmark_dark.pngis excluded by!**/*.pngresources/branding/celune_wordmark_transparent.pngis excluded by!**/*.pngresources/celune.icois excluded by!**/*.icouv.lockis excluded by!**/*.lock
📒 Files selected for processing (68)
.github/workflows/build-linux.yml.github/workflows/build-windows.yml.github/workflows/ci.yml.gitignoreAGENTS.mdGALLERY.mdREADME.mdcelune.AppImagecelune/__init__.pycelune/analysis.pycelune/api.pycelune/backends/base.pycelune/backends/mini.pycelune/backends/qwen3.pycelune/backends/voxcpm2.pycelune/celune.pycelune/cevoice.pycelune/chroma.pycelune/colors.pycelune/config.pycelune/constants.pycelune/entrypoint.pycelune/extensions/base.pycelune/extensions/manager.pycelune/modeling.pycelune/namedays.pycelune/paths.pycelune/persona/impl.pycelune/persona/memory.pycelune/persona/runtime.pycelune/pipeline.pycelune/runtime.pycelune/ui/app.pycelune/ui/commands.pycelune/ui/headless.pycelune/ui/resources.pycelune/ui/terminal.pycelune/ui/theme.pycelune/updater.pycelune/utils.pycelune/vram.pydefault_config.yamllauncher.cmain.pynuitka_main.pypyproject.tomlrequirements.txtresources/celune.resscripts/build_nuitka.ps1scripts/build_nuitka.shscripts/run_ci.pytests/support.pytests/test_analysis_and_chroma.pytests/test_api_audio.pytests/test_api_think.pytests/test_api_webui.pytests/test_backends_and_extensions.pytests/test_backends_mini.pytests/test_celune_core.pytests/test_config_and_utils.pytests/test_main_doctor.pytests/test_modeling.pytests/test_namedays_i18n_updater.pytests/test_persona_api.pytests/test_persona_memory.pytests/test_pipeline.pytests/test_runtime_and_ui_commands.pytests/test_runtime_paths.py
💤 Files with no reviewable changes (2)
- .github/workflows/build-windows.yml
- .github/workflows/build-linux.yml
Comment on lines
+12
to
+13
| $env:CL = "/O2 /GL /GS /guard:cf /DNDEBUG" | ||
| $env:_CL_ = "/link /LTCG /OPT:REF /OPT:ICF /DYNAMICBASE /NXCOMPAT" |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if Nuitka or other tools invoke additional C/C++ compilations
rg -n 'cl\.exe|cl\s|msvc|compiler' scripts/build_nuitka.ps1Repository: celunah/celune
Length of output: 335
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show relevant parts of the script (small enough to be safe)
sed -n '1,140p' scripts/build_nuitka.ps1 | nl -ba
# Find all uses of these env vars
rg -n '\$env:CL|\$env:_CL_' scripts/build_nuitka.ps1
# Find all cl.exe invocations (or other C/C++ compile drivers) in the script
rg -n 'cl\.exe|(^|\s)cl(\s|$)|msvc|compiler' scripts/build_nuitka.ps1
# Check if Nuitka or other tools are invoked in a way that might trigger extra C/C++ compilations
rg -n 'nuitka|pip|subprocess|build|compile|MSVC|vswhere|VsDevCmd|vcvars' scripts/build_nuitka.ps1Repository: celunah/celune
Length of output: 101
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show relevant parts of the script with line numbers (use cat -n instead of nl)
sed -n '1,160p' scripts/build_nuitka.ps1 | cat -n
# Find all uses of these env vars
rg -n '\$env:CL|\$env:_CL_' scripts/build_nuitka.ps1
# Find all cl.exe invocations (or other C/C++ compile drivers) in the script
rg -n 'cl\.exe|(^|\s)cl(\s|$)|msvc|compiler' scripts/build_nuitka.ps1
# Check how/if Nuitka is invoked and whether the script resets env vars
rg -n 'nuitka|pip|subprocess|build|compile|MSVC|vswhere|VsDevCmd|vcvars|$env:CL\s*=|$env:_CL_\s*=' scripts/build_nuitka.ps1Repository: celunah/celune
Length of output: 6746
Scope $env:CL / $env:_CL_ to the launcher build (or explicitly apply them to Nuitka).
In scripts/build_nuitka.ps1, $env:CL and $env:_CL_ are set at lines 12–13 before & uv @arguments`` runs Nuitka (line ~76), so they will affect both Nuitka’s MSVC cl usage and the later manual `cl`/link step in `$compileCmd` (line 98). If these flags are intended only for the launcher, set/restore the env vars just around `cmd /c $compileCmd` instead of leaving them for the whole script.
🤖 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 `@scripts/build_nuitka.ps1` around lines 12 - 13, The environment variables
$env:CL and $env:_CL_ are set globally before the Nuitka invocation (& uv
`@arguments`) so they affect both Nuitka's MSVC invocation and the later manual
compile/link command stored in $compileCmd; scope them only for the launcher
build by setting and restoring them immediately around the launcher invocation
(or explicitly pass equivalent flags to Nuitka) instead of leaving them set for
the whole script—wrap the temporary assignments in a local/try/finally or save
original values, set $env:CL and $env:_CL_ just before running cmd /c
$compileCmd (or before & uv `@arguments` if the intent is for Nuitka only), then
restore the originals after that command completes so $compileCmd runs with the
original environment.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Now THAT is an update. CodeRabbit, do your summary below.
Summary by CodeRabbit
New Features
Documentation
Improvements
Tests