Repository navigation
Add pull request checks for Django, Docker, protobuf and React - #395
Open
jamesmulcahy wants to merge 10 commits into
Open
jamesmulcahy wants to merge 10 commits into
jamesmulcahy wants to merge 10 commits into
Conversation
NSPManager#391 deleted the NSPanelRoomEntitiesPages component when the room editor moved to React, but ten htmx views still built one to render their response. Each raised NameError, so they returned 500 once they reached that line. Nothing in the templates, static JS or React code calls them any more, so remove them, their routes, and handle_entity_modal_result, whose only job was to dispatch to one of them: - partial_remove_entity_from_page_slot - partial_save_edit_entities_page - partial_move_entity - partial_move_entities_pages - partial_delete_entities_page - create_entities_page_in_room - create_or_update_{light,switch,button,scene}_entity - handle_entity_modal_result scenes_post looked lights up through a Light model that no longer exists, so POST /rest/scenes failed for any request with light states. LightState.light points at Entity, so look the light up there. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
docker/ruff.toml never chose which rules to run, so it got whatever the installed ruff enabled by default. ruff 0.8 reports 102 findings on docker/web; 0.16 reports 394, mostly from rules the project never chose. List the long-standing defaults (E4, E7, E9, F) explicitly, and exclude the generated migrations and protobuf modules. Add a "Django checks" workflow that runs a pinned ruff and fails on undefined names and syntax errors (F821, E9). Those found the eleven broken call sites fixed in the previous commit. The remaining findings, mostly unused imports, need a cleanup pass before all of F can be enforced. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The image was only ever built by release.yml, so a broken Dockerfile, Python dependency or MQTTManager cross-compile surfaced when a release was published. Add a "Docker build" workflow that builds the image without pushing it: amd64 and aarch64 on pull requests and pushes, and also armv7, armhf and i386 weekly and on manual runs. The release build compiles every Conan dependency from source, which takes about an hour. Make the Conan package cache a BuildKit cache mount so it can be kept between builds, and save it with actions/cache through buildkit-cache-dance, keyed per architecture on conanfile.py and install_build_tools.sh. Release and local builds behave as before on a cold cache; local rebuilds now reuse the packages. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The ui-builder stage cloned NSPanelManager-Web-UI at whatever its default branch held at build time, so the same commit of this repo could ship a different UI depending on the day it was built, and a UI change reached a release without passing through this repo's CI. Check out a fixed commit, set by the WEB_UI_REF build argument and defaulting to the current head of main (6f3eae63). Shipping a UI change now means bumping WEB_UI_REF here; pass --build-arg WEB_UI_REF=main to build the latest UI locally. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The C++, Python and firmware C code generated from docker/protobuf is committed, so a .proto change can be merged without regenerating it, or regenerated by only one side. Add a "Protobuf" workflow that runs compile_all.sh with protoc 27.0 and protoc-gen-c and fails if the result differs from what is committed. It also checks that protoc (install_protobuf.sh), the Conan protobuf package and the Python protobuf package are the same release. The generated C++ does not compile against a different runtime, and the generated Python does not import with one. All three are 5.27.0 today. compile_all.sh wrote to fixed dev-container paths; its output directories can now be overridden from the environment, with the same defaults. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Add a job to the "Django checks" workflow that runs Django's system checks and makemigrations --check, so a model change without its migration fails on the pull request rather than reaching installs whose database would no longer match the code. Both pass today. settings.py keeps its secret key and database under /data, so the job creates it first. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The React components' build output (dist/ and templates/scripts.html) is committed and served as-is, so a change to src/ that isn't rebuilt, or doesn't type-check, ships unnoticed. Add a "React components" workflow that installs from package-lock.json, runs the build (tsc -b, then vite build), and fails if the output differs from what is committed. The committed build reproduces exactly today. ESLint runs but does not fail the job yet: the existing code has 30 errors, mostly no-explicit-any and react-refresh/only-export-components. package-lock.json is used rather than bun.lock because only the former matches package.json: bun.lock predates the runtime dependencies added since May, and bun install --frozen-lockfile fails on it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
NSPManager#385 added volume_step to NSPanelEntityState.MediaPlayer and regenerated the MQTTManager C++ code, but not the firmware C code in protobuf_firmware/. The new protobuf check caught it. Generated with protoc 27.0 and protoc-gen-c 1.4.1, as CI does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jamesmulcahy
force-pushed
the
ci/pr-checks
branch
from
October 18, 2026 02:30
b939c7c to
0ac05bd
Compare
NSPManager#394 added the Sound message and NSPanelConfig.sounds to protobuf_nspanel.proto but did not regenerate the protobuf-c code the firmware builds against. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jamesmulcahy
marked this pull request as ready for review
October 18, 2026 03:07
As django-tests.yml does, following the review of NSPManager#388: we rarely work in beta directly, but anything that lands there should pass the same checks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
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
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.
Adds pull request checks for the parts of the repo that CI didn't cover: the Django code, the Docker image, the generated protobuf code and the React components. It goes with #388 (Django tests) and #392 (MQTTManager tests). Each check is its own commit, so any of them can be dropped.
Checks
django-checks.yml)docker/web/,docker/ruff.tomlF821,E9);manage.py checkreports a problem; a model change has no migration (makemigrations --check)docker-build.yml)docker/protobuf.yml).protofiles, generated code,conanfile.py,requirements.txtcompile_all.shgenerates; or protoc, the Conan package and the Python package aren't the same protobuf releasereact-components.yml)web/react-components/tsc -borvite buildfails, or the committeddist/andtemplates/scripts.htmldiffer from a fresh build. ESLint runs but doesn't fail the job yet.Fixes and other changes
0473cc36). Ruff'sF821found 11:NSPanelRoomEntitiesPagescomponent, but ten htmx views still built one to render their response. Each raisedNameErrorand returned 500. Nothing in the templates, static JS or React code calls them any more, so this removes them, their routes, andhandle_entity_modal_result, which only dispatched to one of them.scenes_postlooked lights up through aLightmodel that no longer exists, soPOST /rest/scenesfailed for any request with light states. It now looks them up inEntity, filtered to lights.docker/ruff.tomllists its rules. It never setselect, so the rules came from whichever ruff was installed: 0.8 reports 102 findings, 0.16 reports 394. It now lists ruff's long-standing defaults (E4,E7,E9,F) and excludes the generated migrations and protobuf modules. CI pins ruff 0.16.10. 82 findings remain, mostly unused imports, so onlyF821andE9fail the build for now.actions/cacheandbuildkit-cache-dance, one cache per architecture. Cold builds, including releases, behave as before. Local rebuilds now reuse the packages.6fd9f0ce). Theui-builderstage clonedNSPanelManager-Web-UIat whatever itsmainheld at build time. It now checks outWEB_UI_REF, which defaults to the current head ofmain(6f3eae63). Shipping a UI change now means bumping that commit here.--build-arg WEB_UI_REF=mainbuilds the latest UI. This changes how UI updates reach a release, so it's a separate commit and easy to drop.compile_all.sh's output directories can be set from the environment, with the same defaults, so CI can generate into the checkout.Current state
manage.py checkpasses.bun.lockis out of date: it lacks every runtime dependency added since May, andbun install --frozen-lockfilefails.package-lock.jsonmatchespackage.json, so CI installs withnpm ci.no-explicit-anyandreact-refresh/only-export-components.Notes
htmx.pyandurls.py. With the same removal applied on top of Add a Django test harness and initial REST API tests #388, its suite passes (219 tests, 1 expected failure).paths:filters. If any of them becomes a required check, a pull request that doesn't touch those paths will wait for it forever.🤖 Generated with Claude Code