Conversation
📝 WalkthroughWalkthroughThe pull request refactors build infrastructure and tooling configuration by extracting Python version handling to GitHub Actions, relocating customization folder paths, consolidating Makefile targets across Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 1
🤖 Fix all issues with AI agents
In @.rhiza/agentic/Makefile.agentic:
- Around line 24-28: The Makefile uses an undefined color variable `${SUCCESS}`
in the install check printf, causing missing color output; either define SUCCESS
as the intended color alias (e.g., SUCCESS := ${GREEN}) near the other color
vars or replace `${SUCCESS}` with the existing `${GREEN}` in the printf call
inside the copilot install conditional to restore the expected green message
formatting.
🧹 Nitpick comments (1)
.github/workflows/rhiza_docker.yml (1)
54-65: Quote$GITHUB_OUTPUTfor shell safety.While
$GITHUB_OUTPUTis typically a safe path set by GitHub Actions, quoting it protects against edge cases with spaces or special characters.Suggested fix
run: | if [ -f .python-version ]; then PYTHON_VERSION=$(cat .python-version | tr -d '[:space:]') - echo "version=$PYTHON_VERSION" >> $GITHUB_OUTPUT + echo "version=$PYTHON_VERSION" >> "$GITHUB_OUTPUT" echo "Using Python version: $PYTHON_VERSION" else - echo "version=3.12" >> $GITHUB_OUTPUT + echo "version=3.12" >> "$GITHUB_OUTPUT" echo "::warning::.python-version not found, using default 3.12" fi
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.github/workflows/rhiza_docker.yml.rhiza/.env.rhiza/Makefile.rhiza.rhiza/agentic/Makefile.agentic.rhiza/requirements/tools.txtMakefilebook/README.mdtests/test_rhiza/test_makefile.py
🧰 Additional context used
🧬 Code graph analysis (1)
tests/test_rhiza/test_makefile.py (2)
tests/test_rhiza/test_makefile_gh.py (1)
run_make(55-71)tests/test_rhiza/conftest.py (1)
logger(147-153)
🪛 dotenv-linter (4.0.0)
.rhiza/.env
[warning] 4-4: [UnorderedKey] The CUSTOM_SCRIPTS_FOLDER key should go before the MARIMO_FOLDER key
(UnorderedKey)
🔇 Additional comments (11)
book/README.md (1)
24-45: Well-structured documentation addition with clear instructions.The documentation accurately describes pdoc's customizable HTML templates feature, and the instructions for both API documentation and minibook customization are clear and actionable.
However, please verify that the anchor fragment in the link on line 37 is correct. The URL
https://pdoc.dev/docs/pdoc.html#templatesshould be tested to ensure it navigates to the correct section in the pdoc documentation.🔗 Verify the documentation link
Please test the following link to confirm it works correctly:
Based on the pdoc documentation, the correct anchor might be "#edit-pdocs-html-template" instead of "#templates". Verify the link in a browser and update if necessary.
.rhiza/Makefile.rhiza (1)
4-22: LGTM!The RHIZA_LOGO definition and
print-logotarget are correctly structured. Thedefine/endefblock properly handles the multi-line ASCII art, and the export ensures the variable is available to subshells. The$$RHIZA_LOGOescaping is correct for Make to pass the shell variable.Makefile (3)
43-45: LGTM!The Python version extraction logic correctly reads from
.python-versionwith a sensible default of3.12. Using?=allows environment override, andexportensures consistency across sub-makes and recipes.
57-57: LGTM!The optional include of
docker/Makefile.dockerusing-includegracefully handles cases where the file may not exist.
136-137: LGTM!Adding
.benchmarksto the clean target is appropriate alongside.pytest_cachefor removing test artifacts..github/workflows/rhiza_docker.yml (1)
67-78: LGTM!The Docker Buildx step correctly passes the Python version as a build argument, maintaining consistency with the Makefile's single-source-of-truth approach for Python version management.
.rhiza/agentic/Makefile.agentic (1)
7-7: LGTM!The PATH-first resolution for
COPILOT_BINis a good practice, allowing users withcopilotalready in their PATH to use it directly while maintaining backward compatibility with theINSTALL_DIRfallback..rhiza/requirements/tools.txt (1)
4-4: Version typer==0.21.1 is valid and secure.The pinned version exists on PyPI (released Jan 6, 2026) and has no known security vulnerabilities.
.rhiza/.env (1)
1-14: LGTM!The path reorganization from
.rhiza/scripts/customisationsto.rhiza/customisations/scriptsis a sensible structural change that groups customization-related content under a dedicated folder. The change is consistent with the updated test expectations.Regarding the static analysis hint about alphabetical key ordering: the current logical grouping of path-related variables together is reasonable and can be kept as-is.
tests/test_rhiza/test_makefile.py (2)
48-52: LGTM - Good improvement for test determinism.Creating a minimal, deterministic
.rhiza/.envin the test fixture ensures tests don't depend on varying local configurations. The hardcoded values match the actual.rhiza/.envfile, maintaining consistency.
191-195: LGTM!The docstring and assertion are correctly updated to reflect the new
CUSTOM_SCRIPTS_FOLDERpath, and they're consistent with both the fixture setup (line 51) and the actual.rhiza/.envconfiguration.
| @if command -v copilot >/dev/null 2>&1; then \ | ||
| printf "${GREEN}[INFO] copilot already installed in PATH, skipping install.${RESET}\n"; \ | ||
| elif [ -x "${INSTALL_DIR}/copilot" ]; then \ | ||
| printf "${SUCCESS}[INFO] copilot already installed in ${INSTALL_DIR}, skipping install.${RESET}\n"; \ | ||
| else \ |
There was a problem hiding this comment.
Undefined SUCCESS color variable.
Line 27 uses ${SUCCESS} but the main Makefile only defines GREEN, BLUE, RED, YELLOW, and RESET. This will result in an empty string (no color output) rather than the intended green color.
Suggested fix
elif [ -x "${INSTALL_DIR}/copilot" ]; then \
- printf "${SUCCESS}[INFO] copilot already installed in ${INSTALL_DIR}, skipping install.${RESET}\n"; \
+ printf "${GREEN}[INFO] copilot already installed in ${INSTALL_DIR}, skipping install.${RESET}\n"; \📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @if command -v copilot >/dev/null 2>&1; then \ | |
| printf "${GREEN}[INFO] copilot already installed in PATH, skipping install.${RESET}\n"; \ | |
| elif [ -x "${INSTALL_DIR}/copilot" ]; then \ | |
| printf "${SUCCESS}[INFO] copilot already installed in ${INSTALL_DIR}, skipping install.${RESET}\n"; \ | |
| else \ | |
| @if command -v copilot >/dev/null 2>&1; then \ | |
| printf "${GREEN}[INFO] copilot already installed in PATH, skipping install.${RESET}\n"; \ | |
| elif [ -x "${INSTALL_DIR}/copilot" ]; then \ | |
| printf "${GREEN}[INFO] copilot already installed in ${INSTALL_DIR}, skipping install.${RESET}\n"; \ | |
| else \ |
🤖 Prompt for AI Agents
In @.rhiza/agentic/Makefile.agentic around lines 24 - 28, The Makefile uses an
undefined color variable `${SUCCESS}` in the install check printf, causing
missing color output; either define SUCCESS as the intended color alias (e.g.,
SUCCESS := ${GREEN}) near the other color vars or replace `${SUCCESS}` with the
existing `${GREEN}` in the printf call inside the copilot install conditional to
restore the expected green message formatting.
1 participant
This pull request synchronizes the repository with its template.
Changes were generated automatically using rhiza.
Summary by CodeRabbit
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.