Skip to content

Backport vacuum and some vrt utilities and pull in base template - #201

Merged
zendern merged 18 commits into
mainfrom
backport-vacuum-from-git-sherpa
Jul 26, 2026
Merged

Backport vacuum and some vrt utilities and pull in base template#201
zendern merged 18 commits into
mainfrom
backport-vacuum-from-git-sherpa

Conversation

@zendern

@zendern zendern commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Link to Issue or Message thread

Why is this change necessary?

Backporting and general updating from base template

How does this change address the issue?

  • Backports all vacuum tweaks we made downstream into the template
  • Pulls in latest copier-base-template
  • Backports some vrt helper method that we have been using downstream

What side effects does this change have?

N/A

How is this change tested?

CI and downstream

Summary by CodeRabbit

  • API Improvements

    • API schemas now consistently present camelCase field names with clearer titles and examples.
    • Error responses use standardized Problem Details formatting with improved OpenAPI documentation.
    • Health check and shutdown responses provide more accurate, discoverable schema metadata.
  • Validation

    • OpenAPI validation now correctly recognizes descriptions and examples defined through referenced or nested schemas.
    • Additional API design rules are enforced to improve consistency across generated services.
  • Testing

    • Visual checks now cover both light and dark themes.

zendern added 16 commits July 24, 2026 22:50
The 0.2.x line adds the ShellRail and PageTitle shell components plus the
shellRailTestIds helper, which the circuit-python device driver frontend
template uses for the shared Lab Sync theming.

0.2.3 declares `@nuxt/ui >=4.10.0` and a non-optional `openapi-types >=12.1`
peer dependency, so bump @nuxt/ui and add openapi-types as a dev dependency
for driver projects.
…aware ones

The built-in operation-description and oas3-missing-example rules inspect
requestBody/response nodes directly and never follow $ref, so a pydantic
model's docstring and its field-level examples are invisible to them. That
forced every route to hand-type a description and example duplicating what
the payload model already declares.

Disable both and replace them with request/response-body-description and
request/response-body-example, backed by two new custom functions that run
against the schema vacuum has already dereferenced.

Also raise every self-defined rule to error severity, and move the graphql
200-response exemption over to the new response-body-example rule.

Backported from lab-sync/git-sherpa#241.
…linters

The hook interpolated vacuum_openapi_version, which was never defined in
copier.yml or the context extension, so it rendered empty and downstream
repos installed an unpinned '@quobix/vacuum@'. Define it alongside the other
tool pins.

Also move the hook out of the tail of the has_backend block, where it sat
behind pyright and pyrefly, to just after the security hooks. It is cheap
and its failures are cheap to act on, so it should fail before the slow
type-checkers run.
Every downstream app needs the same camelCase-over-the-wire base model and
was reinventing it, so ship it in the template. HealthcheckResponse,
HealthcheckQuery, ShutdownResponse and ProblemDetails all base on it now.

ProblemDetails carried a hand-written alias="errorType" to camelCase one
field; the alias generator covers that now. Dropping the explicit alias also
lets field_title_generator apply, so its JSON-schema title becomes "Error
Type" instead of the "Errortype" pydantic derives from an alias. The wire
format is unchanged.

The healthcheck query parameter moves from a bare Query(alias="prependV") to
a HealthcheckQuery model, which drops a pyright suppression and gets the
alias from the generator. Response fields gain examples so the new
request/response-body-example rules pass on the model rather than on a
hand-typed duplicate.

Snapshot updated to match, and verified against the schema the rendered app
actually serves.

Backported from lab-sync/git-sherpa#241.
…sponses

Documenting an HTTPException in a route decorator meant hand-writing the
same application/problem+json media type and ProblemDetails schema ref every
time. problem_response holds that shared part so a route passes only what
differs: status code and description.

It also synthesizes an example from those two, so the docs show a body
matching that specific response instead of the generic field-level example a
bare schema ref renders.

Backported from lab-sync/git-sherpa#241, with tests added.
…mentScreenshot

expectContentPaneScreenshot assumed the main area held no dynamic chrome, so
it took no masks. That does not hold once a page renders non-deterministic
values (generated ids and the like), so accept a mask list like the full-page
helpers already do.

expectElementScreenshot screenshots a single locator rather than the page or
the content pane, keeping a widget's baseline insensitive to unrelated
layout churn elsewhere. Use it for a self-contained section that a broader
page-level VRT already covers for overall layout. It takes masks too --
LocatorAssertions.toHaveScreenshot supports the same option -- so all four
helpers now expose it consistently.

Backported from lab-sync/git-sherpa#241 and lab-sync/sensor-monitor#148.
@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The PR adds schema-aware Vacuum body description and example checks, strengthens OpenAPI lint severities and tooling, introduces camelCase backend models and problem-response generation, updates OpenAPI snapshots and tests, and extends visual regression helpers for light/dark screenshot baselines and masking.

Changes

OpenAPI linting

Layer / File(s) Summary
Schema-aware body validation
template/.config/vacuum-functions/*
Custom Vacuum rules inspect body descriptions and examples across media objects and dereferenced schemas.
Ruleset and GraphQL exemptions
template/.config/vacuum-ruleset.yaml, template/.config/vacuum-ignore.yaml.jinja
Built-in body checks are replaced with custom rules, selected severities become errors, and GraphQL response exemptions are updated.
Vacuum execution wiring
template/.pre-commit-config.yaml.jinja, template/.gitignore
The backend pre-commit configuration runs Vacuum with pinned dependencies, while generated reports are ignored.

Backend API contracts

Layer / File(s) Summary
CamelCase models and healthcheck query
template/.../backend_api/camel_case_model.py, template/.../backend_api/app_def.py.jinja, template/.../tests/unit/test_camel_case_model.py, template/.../tests/unit/__snapshots__/*
Backend schemas use camelCase serialization with snake_case-derived titles, and healthcheck query handling uses HealthcheckQuery.
Problem details serialization and OpenAPI responses
template/.../backend_api/fast_api_exception_handlers.py, template/.../backend_api/openapi_problem_responses.py
ProblemDetails uses CamelCaseModel, and problem_response generates problem+json OpenAPI response entries with examples.
Backend contract validation
template/.../tests/unit/test_fast_api_exception_handlers.py, template/.../tests/unit/test_openapi_problem_responses.py
Tests validate schema naming, problem response content and examples, and CORS headers.

Frontend visual test helpers

Layer / File(s) Summary
Masked and themed screenshot capture
template/frontend/tests/e2e/helpers/vrt.ts
Screenshot helpers iterate over color schemes, use scheme-suffixed names, support mask arrays, and capture the navigation rail with a default logo mask.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

Suggested reviewers: ejfine

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and matches the main change: backporting vacuum updates and VRT utilities into the template.
Description check ✅ Passed The description follows the required template and covers the main sections, with only the optional "Other" section omitted.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

❤️ Share

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

@zendern
zendern marked this pull request as ready for review July 26, 2026 01:17
@zendern
zendern requested a review from ejfine July 26, 2026 01:17
Comment thread template/.gitignore
description: Path parameters must be snake_case.
given: $.paths[*][*].parameters[?(@.in=='path')].name
severity: warn
severity: error

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

NIT: do we have documentation somewhere of why we're changing these all to error (e.g. a link to the github issue you created)?

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.

Issue here daveshanley/vacuum#947 but TLDR; output is a little weird if they are not errors. So at least the ones that we control here we are bumping them up to error even though our fail-severity is warning so that if any of those fail they render as such.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I know right now why we're doing it. I meant "should we add a note or link to the issue in the the code for our future selves?". Just nit though

page: Page,
name: string,
{ colorSchemes = ["light", "dark"] }: { colorSchemes?: ColorScheme[] } = {},
{ mask = [], colorSchemes = ["light", "dark"] }: { mask?: Locator[]; colorSchemes?: ColorScheme[] } = {},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

NIT: should it be masks plural, since it's an array? not sure if there's a different playwright convention

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.

Updated ff4b0df

ejfine
ejfine previously approved these changes Jul 26, 2026
@zendern
zendern merged commit 45470d0 into main Jul 26, 2026
25 checks passed

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🤖 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 `@template/.config/vacuum-functions/bodyDescription.js`:
- Around line 25-44: Update schemaHasDescription to traverse an array schema’s
items in addition to anyOf/oneOf branches, so descriptions reachable through
list item models are detected. Add cycle protection using a seen-schema tracking
array, matching the established approach in bodyExample.js, and ensure recursive
schemas terminate safely while preserving the existing direct-description and
branch checks.

In `@template/.config/vacuum-functions/bodyExample.js`:
- Around line 19-27: Update hasOwnExample to recognize Media Type Object
examples maps in addition to schema-style examples arrays: treat a non-null
object-valued node.examples as present when it is a named map, while preserving
the existing array and node.example checks.

In `@template/`{% if has_backend %}backend{% endif
%}/src/backend_api/openapi_problem_responses.py:
- Around line 27-28: Update the example construction in problem_response to
handle integer status codes not registered in HTTPStatus, such as 499 or 599.
Either restrict the function’s accepted status-code contract to registered HTTP
statuses or add explicit validation/fallback behavior before accessing
HTTPStatus(status_code), while preserving normal phrase generation for
registered codes.

In `@template/`{% if has_backend %}backend{% endif
%}/tests/unit/test_fast_api_exception_handlers.py:
- Around line 143-144: Update the CORS exception-handler test request setup to
include an allowed Origin header before checking response.headers. In the
assertions following the request, validate the expected
Access-Control-Allow-Origin value and Access-Control-Allow-Credentials value
rather than only checking header presence.

In `@template/frontend/tests/e2e/helpers/vrt.ts`:
- Around line 103-108: Update pushLogoMask in vrt-masks.ts so it adds the
intended logo Locator to the provided defaultMasks array. Preserve the existing
caller in the colorScheme screenshot loop and ensure defaultMasks contains the
logo mask before it is passed to toHaveScreenshot.
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: f7a901bd-1565-4842-8cd9-28feacf5a023

📥 Commits

Reviewing files that changed from the base of the PR and between 6734146 and ff4b0df.

📒 Files selected for processing (15)
  • template/.config/vacuum-functions/bodyDescription.js
  • template/.config/vacuum-functions/bodyExample.js
  • template/.config/vacuum-ignore.yaml.jinja
  • template/.config/vacuum-ruleset.yaml
  • template/.gitignore
  • template/.pre-commit-config.yaml.jinja
  • template/frontend/tests/e2e/helpers/vrt.ts
  • template/{% if has_backend %}backend{% endif %}/src/backend_api/app_def.py.jinja
  • template/{% if has_backend %}backend{% endif %}/src/backend_api/camel_case_model.py
  • template/{% if has_backend %}backend{% endif %}/src/backend_api/fast_api_exception_handlers.py
  • template/{% if has_backend %}backend{% endif %}/src/backend_api/openapi_problem_responses.py
  • template/{% if has_backend %}backend{% endif %}/tests/unit/__snapshots__/test_basic_server_functionality/test_openapi_schema.json.jinja
  • template/{% if has_backend %}backend{% endif %}/tests/unit/test_camel_case_model.py
  • template/{% if has_backend %}backend{% endif %}/tests/unit/test_fast_api_exception_handlers.py
  • template/{% if has_backend %}backend{% endif %}/tests/unit/test_openapi_problem_responses.py

Comment thread template/.config/vacuum-functions/bodyDescription.js
Comment thread template/.config/vacuum-functions/bodyExample.js
Comment thread template/frontend/tests/e2e/helpers/vrt.ts
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