Skip to content

Add pull request checks for Django, Docker, protobuf and React - #395

Open
jamesmulcahy wants to merge 7 commits into
NSPManager:develfrom
jamesmulcahy:ci/pr-checks
Open

jamesmulcahy wants to merge 7 commits into
NSPManager:develfrom
jamesmulcahy:ci/pr-checks

Conversation

@jamesmulcahy

@jamesmulcahy jamesmulcahy commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Adds pull request checks for the parts of the repo CI didn't cover yet: the Django code, the Docker image, the generated protobuf code and the React components. With #388 (Django tests) and #392 (MQTTManager tests), every part of the repo is now checked on each pull request. Each check is its own commit, so any of them can be dropped.

The checks have already paid for themselves. While they were being written they found 11 broken call sites in the Django views, and three .proto changes (#385, #394, #405) whose firmware C code was never regenerated. All of these are fixed here, and every check passes on this branch.

Checks

Workflow Runs on changes to Fails when
Django checks (django-checks.yml) docker/web/, docker/ruff.toml ruff finds an undefined name or syntax error (F821, E9); manage.py check reports a problem; a model change has no migration (makemigrations --check)
Docker build (docker-build.yml) docker/ The release image doesn't build. Pull requests and pushes build amd64 and aarch64. A weekly run, which can also be started by hand, adds armv7, armhf and i386. Nothing is pushed.
Protobuf (protobuf.yml) .proto files, generated code, conanfile.py, requirements.txt The committed C++, Python or firmware C code differs from what compile_all.sh generates; or protoc, the Conan package and the Python package aren't the same protobuf release
React components (react-components.yml) web/react-components/ tsc -b or vite build fails, or the committed dist/ and templates/scripts.html differ from a fresh build. ESLint runs but doesn't fail the job yet.

All four run on pull requests and on pushes to main, devel and beta.

Regenerating protobuf code

docker/protobuf/regenerate.sh is new. It regenerates all the committed protobuf code in a throwaway Ubuntu container and needs only Docker. Before, the only way was the full dev container, which takes a Conan build to create. The protobuf check runs the same script, so when it fails, its error message says to run regenerate.sh and commit the result, and that fixes it.

Fixes found by the checks

Other changes

  • docker/ruff.toml lists its rules. It never set select, 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 for now only F821 and E9 fail the build. The rest can be cleaned up in a later pass.
  • Faster Docker builds. The release build compiles every Conan dependency from source, which takes about an hour. The Conan package cache is now a BuildKit cache mount, and CI keeps it between runs (actions/cache with buildkit-cache-dance, one cache per architecture). Local rebuilds reuse the packages too. Cold builds, including releases, work as before. A new push to a pull request cancels that pull request's running image build.
  • Choosing which Web UI to build. A new WEB_UI_REF build argument picks the NSPanelManager-Web-UI branch, tag or commit to build, e.g. --build-arg WEB_UI_REF=devel. It defaults to main, so releases still ship whatever the Web UI's main holds.
  • compile_all.sh's output directories can be set from the environment, with the same defaults as before.

Current state

  • The committed React build and the generated C++/Python protobuf code match a fresh build exactly. There are no missing migrations, and manage.py check passes.
  • bun.lock is out of date: it's missing every runtime dependency added since May, and bun install --frozen-lockfile fails. package-lock.json matches package.json, so CI installs with npm ci.
  • ESLint reports 30 errors, mostly no-explicit-any and react-refresh/only-export-components. Once those are fixed, it can be made blocking.

Notes

  • Required checks. All the workflows use paths: filters. If any of them becomes a required check, a pull request that doesn't touch those paths will wait for it forever.
  • Docker build cache. The first build on each architecture fills the Conan cache, which takes about an hour. Builds after that reuse it.

🤖 Generated with Claude Code

@@ -0,0 +1,91 @@
name: "Docker build"

# Builds the release image without pushing it, so a broken Dockerfile, dependency or cross-compile

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been thinking about this. Maybe it wouldn't be bad to build a nightly release if things has changed for developers to more easily test things out before release. Especially now with conan caching I see this as a possibility. Really nice work, thanks a ton!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! A nightly build would be a neat thing to build on top of this. With the Conan cache in place it should be cheap enough to only build when devel has changed, and it would make it much easier for people to try things out before a release.


REPO_ROOT=$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)

# Ubuntu 24.04's protobuf-c-compiler is 1.4.1; install_protobuf.sh installs protoc 27.0.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know of any better library than protc? As protc has some limitations and it's stuck on an old version of protobuf and no longer seems maintained. Some things aren't implemented, such as "optional" and it makes it so that we have to manually control "has_*" flags in the protobuf and cannot update protobuf generally to get bug fixes and other things.

Comment thread docker/Dockerfile
# The Conan package cache (core.cache:storage_path in install_build_tools.sh) is a BuildKit cache
# mount, so rebuilds reuse the dependencies instead of compiling them from source again, and CI can
# save it between runs. Only /MQTTManager/build is copied out of this stage.
RUN --mount=type=cache,id=nspm-conan,target=/MQTTManager/conan_cache,sharing=locked \

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This to me looks a bit malformed, --mount=type=cache should probably be mount --type=cache?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for looking at this closely! It does look unusual, but this one is valid as written. --mount is a flag on the RUN instruction itself, part of BuildKit's Dockerfile syntax (docs). type=cache mounts /MQTTManager/conan_cache as a cache directory that persists between builds but is never included in the image, so the Conan dependencies are reused instead of being rebuilt each time.

BuildKit is the default builder in current Docker, and CI builds with docker/setup-buildx-action + docker/build-push-action. buildkit-cache-dance moves the actions/cache copy into that same mount, which is how CI gets to reuse it between runs.

jamesmulcahy and others added 7 commits October 21, 2026 22:51
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>
Two .proto changes regenerated the MQTTManager C++ code but not the
protobuf-c code in protobuf_firmware/ that the firmware builds against:

- NSPManager#394 added the Sound message and NSPanelConfig.sounds
- NSPManager#405 added NSPanelConfig.tz_posix

The protobuf check added later in this series caught both.
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>
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.

The "Django checks" workflow has two jobs:

- ruff, pinned, failing 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.
- 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 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 always cloned NSPanelManager-Web-UI's default
branch. Add a WEB_UI_REF build argument that takes a branch, tag or
full commit hash, so a build can use the Web UI's devel branch or a
fixed commit. It defaults to main, so releases and existing builds are
unchanged.

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 regenerates
it 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.

Regenerating used to need the dev container, which takes a full Conan
build to create. docker/protobuf/regenerate.sh runs compile_all.sh in a
plain Ubuntu 24.04 container with protoc 27.0 and protoc-gen-c 1.4.1,
and needs nothing but Docker. The workflow runs that same script, so a
failing check is fixed by running it and committing the result, and the
error message says so. compile_all.sh's output directories can now be
set from the environment, with the same defaults.

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>

This branch has not been deployed

No deployments
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.

2 participants