Add beginner-friendly web config generator with PR previews#399
Open
tomquist wants to merge 18 commits into
Open
Add beginner-friendly web config generator with PR previews#399tomquist wants to merge 18 commits into
tomquist wants to merge 18 commits into
Conversation
A static, data-driven site (web/) that walks users through a few questions about their power meter and generates either a Python config.ini or an ESPHome YAML, with per-option help text, a live preview, and save/load/share of the UI state. Deployed via a new GitHub Pages workflow. - web/js/schema.js: single source of truth for every powermeter and option - web/js/generate.js: pure config.ini / ESPHome YAML generators (Node-testable) - web/js/app.js: progressive-disclosure form, live preview, localStorage + project file + share-link persistence - web/js/generate.test.mjs: assertions run in CI before deploy - .github/workflows/pages.yml: test + deploy web/ to GitHub Pages
Switch the Pages deploy to publish web/ to a gh-pages branch (keep_files so PR previews survive), and add a pr-preview workflow that deploys each PR to pr-preview/pr-<number>/ and comments the live URL, so the site can be tested directly from the PR.
WalkthroughThis PR introduces a fully in-browser configuration generator for AstraMeter that produces ChangesWeb Configuration Generator
🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 8
🧹 Nitpick comments (1)
web/js/generate.test.mjs (1)
112-153: ⚡ Quick winAdd regression assertions for multi-phase filters and dual-MQTT configuration
Please add explicit tests for:
- 3-phase ESPHome output applying offset/multiply filters to
grid_l1,grid_l2, andgrid_l3.- MQTT meter + MQTT Insights together, including broker mismatch behavior (either validated or intentionally canonicalized).
🤖 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 `@web/js/generate.test.mjs` around lines 112 - 153, Add two regression assertions: for the 3-phase ESPHome case (eyHa from generateEsphome) assert that filters for grid_l1, grid_l2 and grid_l3 include the offset/multiply lines (e.g., check for "- offset: -20" and any multiply filter strings applied to grid_l1/grid_l2/grid_l3 using has(eyHa, ...)). For the MQTT case (eyMqtt from generateEsphome) add assertions that the mqtt_insights block and marstek_registration block both appear and explicitly assert broker/topic behavior when MQTT meter and mqttInsights are both set (e.g., check presence of "broker: 192.168.1.10" or whatever canonicalized broker string you expect, and "mqtt_insights:" plus "platform: mqtt_subscribe" / "topic: home/p") to cover broker mismatch/canonicalization scenarios.
🤖 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 @.github/workflows/pages.yml:
- Around line 32-45: Pin the third-party GitHub Actions to immutable commit SHAs
and disable checkout credential persistence: replace the mutable tags
actions/checkout@v4, actions/setup-node@v4, and peaceiris/actions-gh-pages@v4
with their corresponding commit SHAs, and add persist-credentials: false to the
actions/checkout step (the checkout step referenced as uses: actions/checkout).
Ensure the other action inputs remain unchanged while swapping tags for the SHA
pins.
In @.github/workflows/pr-preview.yml:
- Around line 33-41: Update the workflow steps that use actions/checkout,
actions/setup-node, and rossjrw/pr-preview-action to pin them to their full
commit SHAs (replace the mutable tags like `@v4` and `@v1` with the corresponding
full SHA strings), and add persist-credentials: false under the actions/checkout
step so the checkout step does not persist GitHub credentials; ensure you modify
the steps referencing actions/checkout, actions/setup-node, and
rossjrw/pr-preview-action accordingly (and keep the node-version: "20" and the
existing Run config generator tests and Deploy preview step logic intact).
In `@CHANGELOG.md`:
- Line 5: Consolidate the multiple bullets under the "## Next" section into a
single summary bullet by folding the new sentence about the beginner-friendly
web-based config generator into that one overall item; locate the "## Next"
header and replace the separate bullet containing the line about "a
beginner-friendly web-based config generator (`web/`, deployable to GitHub
Pages)..." with a concise single summary bullet that captures that feature plus
any other pending items for this branch, merging/editting (not appending) so
there remains exactly one bullet under "## Next".
In `@web/js/app.js`:
- Around line 441-451: copyConfig() and downloadConfig() call generate(state)
directly so a thrown generation error can crash the action with no user
feedback; wrap the generate(state) call in a try/catch in both functions, on
success proceed (for copyConfig: call
navigator.clipboard.writeText(...).then(...) as before; for downloadConfig: call
downloadText(name, text)); on error call toast with an error message (e.g.,
"Generation failed" and mark as error) and avoid calling clipboard/download
functions. Reference functions: copyConfig, downloadConfig, generate, toast,
downloadText.
- Around line 453-459: The downloadText function creates an object URL with
URL.createObjectURL(blob) but never revokes it, leaking resources; update
downloadText to store the object URL in a variable (e.g., const url =
URL.createObjectURL(blob)), assign it to the anchor href, then after triggering
the download call URL.revokeObjectURL(url) (use a short setTimeout (e.g.,
0-1000ms) or the anchor's click event callback to revoke so the download has
time to start), and ensure you still remove the anchor element; reference:
downloadText, blob, a, URL.createObjectURL, URL.revokeObjectURL.
- Around line 138-143: The password inputs are being forced to "text" by the
expression type: field.type === "password" ? "text" : field.type; in the code
that creates the input element (look for control creation using field.type and
the variable control), so change it to preserve the original field.type (i.e.,
use type: field.type) so password fields render with type="password"; keep the
existing secret class addition (control.classList.add("secret") when field.type
=== "password") and leave the input event handling
(control.addEventListener("input", ...)) as-is.
In `@web/js/generate.js`:
- Around line 193-200: The current filter generation builds filterLines and
filterBlock using tuning.POWER_OFFSET, tuning.POWER_MULTIPLIER and
tuning.THROTTLE_INTERVAL but only emits them for the first sensor (L1); update
the logic that builds and inserts filterLines/filterBlock (symbols: filterLines,
filterBlock, tuning.POWER_OFFSET, tuning.POWER_MULTIPLIER,
tuning.THROTTLE_INTERVAL, IND) so it constructs and injects the same filter
block for every phase sensor (L1/L2/L3) when in 3-phase mode instead of
reusing/placing it only once; ensure you apply the same fix to the other similar
blocks mentioned (the instances around the other occurrences) so each phase
sensor gets identical offset/multiply/throttle entries.
- Around line 447-452: The mqtt insights code currently unconditionally emits
the insights broker/port when wantInsights is true, which can override a meter’s
own mqtt settings (esp.kind === "mqtt"); update the generate logic to detect
when a meter has esp.kind === "mqtt" and both meter-level mqtt config and
state.mqttInsights are present, then either (a) compare mf.BROKER/mf.PORT with
the meter’s broker/port and emit a processLogger.warn or throw if they differ,
or (b) deterministically merge by selecting a canonical source (e.g., prefer
meter-specific values and fall back to mf), and write the merged broker/port to
out (use the existing wantInsights, mf and esp.kind checks and the same out.push
path so output remains explicit and deterministic).
---
Nitpick comments:
In `@web/js/generate.test.mjs`:
- Around line 112-153: Add two regression assertions: for the 3-phase ESPHome
case (eyHa from generateEsphome) assert that filters for grid_l1, grid_l2 and
grid_l3 include the offset/multiply lines (e.g., check for "- offset: -20" and
any multiply filter strings applied to grid_l1/grid_l2/grid_l3 using has(eyHa,
...)). For the MQTT case (eyMqtt from generateEsphome) add assertions that the
mqtt_insights block and marstek_registration block both appear and explicitly
assert broker/topic behavior when MQTT meter and mqttInsights are both set
(e.g., check presence of "broker: 192.168.1.10" or whatever canonicalized broker
string you expect, and "mqtt_insights:" plus "platform: mqtt_subscribe" /
"topic: home/p") to cover broker mismatch/canonicalization scenarios.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1877e289-91b7-4785-968e-19d8e24d4423
📒 Files selected for processing (11)
.github/workflows/pages.yml.github/workflows/pr-preview.ymlCHANGELOG.mdREADME.mdweb/README.mdweb/css/styles.cssweb/index.htmlweb/js/app.jsweb/js/generate.jsweb/js/generate.test.mjsweb/js/schema.js
|
|
||
| ## Next | ||
|
|
||
| - **Added** a beginner-friendly web-based config generator (`web/`, deployable to GitHub Pages) that walks you through a few questions about your power meter and produces a ready-to-use Python `config.ini` or ESPHome YAML, with a live preview and save/load/share of your answers. |
There was a problem hiding this comment.
Consolidate ## Next to a single summary bullet.
Line 5 adds/edits one entry, but ## Next now contains multiple bullets. Please keep exactly one overall summary bullet for this branch and fold this text into that single item.
As per coding guidelines, CHANGELOG.md should “keep one bullet under ## Next ... and edit (not append) on later iterations if scope shifts.”
🤖 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 `@CHANGELOG.md` at line 5, Consolidate the multiple bullets under the "## Next"
section into a single summary bullet by folding the new sentence about the
beginner-friendly web-based config generator into that one overall item; locate
the "## Next" header and replace the separate bullet containing the line about
"a beginner-friendly web-based config generator (`web/`, deployable to GitHub
Pages)..." with a concise single summary bullet that captures that feature plus
any other pending items for this branch, merging/editting (not appending) so
there remains exactly one bullet under "## Next".
Owner
Author
There was a problem hiding this comment.
This is already compliant with the guideline as written: the rule is "keep one bullet under ## Next that summarizes the overall outcome of that branch" — i.e. one bullet per branch. This PR adds exactly one bullet for its branch; the other bullets under ## Next are from previously-merged branches and shouldn't be folded together (they describe unrelated changes already on develop). So I'm leaving this as-is.
Generated by Claude Code
There was a problem hiding this comment.
@tomquist, you're right — I misread the guideline. The rule is one bullet per branch under ## Next, not one consolidated bullet for all branches. This PR correctly adds a single bullet for its own branch, and the other bullets belong to independent branches already on develop. My comment was incorrect; sorry for the noise!
✏️ Learnings added
Learnt from: tomquist
Repo: tomquist/AstraMeter PR: 399
File: CHANGELOG.md:5-5
Timestamp: 2026-05-28T19:46:43.804Z
Learning: In the AstraMeter repository (`CHANGELOG.md`), the project convention for the `## Next` section is **one bullet per branch** (not one bullet total). Each branch/PR contributes exactly one bullet summarizing that branch's overall outcome. Multiple bullets under `## Next` are expected when multiple branches have been merged to `develop` and are awaiting release. Reviewers should not flag this as a violation.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
…/download - app.js: render password fields as type=password (were plain text); guard copy/download against generator errors; revoke object URLs after download - generate.js: apply offset/multiply/throttle filters to every phase sensor in 3-phase ESPHome output (single value applies to all phases, per-phase list maps per phase) — previously only L1 was filtered; warn and reuse the meter's broker when an MQTT meter and MQTT Insights share ESPHome's single mqtt: block - tests: cover per-phase filter mapping
Contributor
|
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/js/generate.js (1)
484-492:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDuplicate
http_request:block when HTTP meter and Marstek are both enabled.HTTP-based meters (Shelly, json_http) add
http_request:totopBlocksat line 273. When Marstek is also enabled, anotherhttp_request:block is added at line 485. ESPHome allows only one top-levelhttp_request:component, so this produces invalid YAML.Apply the same deduplication pattern used for
mqtt:.Proposed fix
+ // http_request: is needed by Marstek *or* HTTP-based meters; emit once. + const meterUsesHttp = topBlocks.some((b) => b.startsWith("http_request:")); if (wantMarstek) { - out.push(`http_request:\n${IND}timeout: 20s`); + if (!meterUsesHttp) { + out.push(`http_request:\n${IND}timeout: 20s`); + } } // top blocks from the sensor (api/mqtt/uart/etc.) — de-dup api/mqtt if already added for (const b of topBlocks) { if (wantInsights && b.startsWith("mqtt:")) continue; + // http_request: already emitted for Marstek with longer timeout + if (wantMarstek && b.startsWith("http_request:")) continue; out.push(b); }🤖 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 `@web/js/generate.js` around lines 484 - 492, When Marstek is enabled you always push a second http_request: block causing duplicate top-level components; change the Marstek branch so it only pushes the http_request: block if topBlocks does not already contain an http_request entry. Concretely, in the wantMarstek branch check topBlocks.some(b => b.startsWith("http_request:")) (use the same startsWith pattern used for mqtt) and only call out.push(`http_request:\n${IND}timeout: 20s`) when that check is false; keep the existing IND and out variables and leave the existing for-loop handling of topBlocks/wantInsights unchanged.
🤖 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.
Outside diff comments:
In `@web/js/generate.js`:
- Around line 484-492: When Marstek is enabled you always push a second
http_request: block causing duplicate top-level components; change the Marstek
branch so it only pushes the http_request: block if topBlocks does not already
contain an http_request entry. Concretely, in the wantMarstek branch check
topBlocks.some(b => b.startsWith("http_request:")) (use the same startsWith
pattern used for mqtt) and only call out.push(`http_request:\n${IND}timeout:
20s`) when that check is false; keep the existing IND and out variables and
leave the existing for-loop handling of topBlocks/wantInsights unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fa15ec93-cc4e-46b0-ace8-c01953f6a8cf
📒 Files selected for processing (3)
web/js/app.jsweb/js/generate.jsweb/js/generate.test.mjs
🚧 Files skipped from review as they are similar to previous changes (2)
- web/js/generate.test.mjs
- web/js/app.js
For the ESPHome target the UI now explains what ESPHome is (assuming zero prior knowledge), recommends the ESP32-S3 DevKitC-1 with purchase links, warns that the board selection must match the hardware, and adds a step-by-step 'how to flash this' card (install ESPHome, replace the file, set WiFi secrets, match the board, flash over USB, point the battery at it). The generated YAML header carries the same numbered quick-start. Default board is now the recommended ESP32-S3 DevKitC-1.
|
Actionable comments posted: 0 |
Add a marketing landing page (index.html) with hero, feature grid, supported devices, a power-meter grid and feature list rendered from the shared schema, how-it-works, installation options, FAQ, and a shared sticky nav + footer. Move the config generator to generator.html (linked from the nav and hero). Add js/site.js for shared nav behaviour and schema-driven landing content, and extend styles.css with the site/nav/footer/landing design. Update docs and the changelog bullet to describe the broader website.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
web/js/site.js (1)
55-62: ⚡ Quick winTier-label lookup map matches all
esphome.tiervalues inweb/js/schema.js
web/js/schema.jsonly declaresesphome.tierasnative | generic | alternate | unsupported, so the current 4-key lookup won’t produceundefinedbadges for any schema entries.Optional future-proofing
- const tierLabel = { native: "ESP32 native", generic: "ESP32 via HTTP", alternate: "ESP32 alt.", unsupported: "Python only" }[tier]; + const tierLabel = { native: "ESP32 native", generic: "ESP32 via HTTP", alternate: "ESP32 alt.", unsupported: "Python only" }[tier] || "Python only";🤖 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 `@web/js/site.js` around lines 55 - 62, The lookup for tierLabel and tierCls currently assumes pm.esphome.tier will always be one of the four known values, which is true today but could break on new values; update the code around POWERMETERS iteration (use the pm.esphome.tier value) to provide safe fallbacks: replace the direct map access with a const tierMap = { native: "...", generic: "...", alternate: "...", unsupported: "..." }; then compute const tierLabel = tierMap[tier] ?? "Unknown tier"; and compute tierCls with a default branch (e.g. const tierCls = tier === "unsupported" ? "bad" : tier === "alternate" ? "warn" : tier === "native" || tier === "generic" ? "ok" : "unknown"); this keeps existing behavior for known tiers while preventing undefined badges for unexpected pm.esphome.tier values.
🤖 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.
Nitpick comments:
In `@web/js/site.js`:
- Around line 55-62: The lookup for tierLabel and tierCls currently assumes
pm.esphome.tier will always be one of the four known values, which is true today
but could break on new values; update the code around POWERMETERS iteration (use
the pm.esphome.tier value) to provide safe fallbacks: replace the direct map
access with a const tierMap = { native: "...", generic: "...", alternate: "...",
unsupported: "..." }; then compute const tierLabel = tierMap[tier] ?? "Unknown
tier"; and compute tierCls with a default branch (e.g. const tierCls = tier ===
"unsupported" ? "bad" : tier === "alternate" ? "warn" : tier === "native" ||
tier === "generic" ? "ok" : "unknown"); this keeps existing behavior for known
tiers while preventing undefined badges for unexpected pm.esphome.tier values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4f37e7f3-95a4-4ac3-b24c-ae08fb80aae3
📒 Files selected for processing (7)
CHANGELOG.mdREADME.mdweb/README.mdweb/css/styles.cssweb/generator.htmlweb/index.htmlweb/js/site.js
✅ Files skipped from review due to trivial changes (2)
- README.md
- web/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- CHANGELOG.md
- Remove the last pm.id special-cases from generate.js; meter-specific ESPHome behaviour is now declarative on the schema (esphome.warn, esphome.headersField, esphome.haEntity), so editing/adding a meter is a one-file change in schema.js - Add schema.test.mjs: structural validation that fails on typos, bad field types, selects without options, unknown esphome.kind, duplicate sections, and stray property names; wired into both Pages workflows - Add JSDoc typedefs (Field / EsphomeSpec / Powermeter) for editor autocomplete - Document 'Adding or editing a powermeter' in web/README with a worked example - Cover the declarative paths with new generator assertions
…able XSS audit found no exploitable issue: every restored/user value reaches only textContent, input .value, or setAttribute — never an HTML sink. This adds defense-in-depth so the 'safe today' invariant can't silently rot: - Extract the pure state model + persistence helpers into web/js/state.js (defaultState, migrate, cleanMeter, prototype-pollution-safe safeParse) - safeParse drops __proto__/constructor/prototype keys on share-link/project restore; migrate constrains restored input: meter.type -> known id, target -> python|esphome, phases -> 1|3, string/object coercion of value-bearing fields - Add web/js/state.test.mjs (runs in plain Node) covering the hostile cases; wire it into both Pages workflows - Comment the one interpolated innerHTML site noting only static schema constants may be interpolated there
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@web/js/state.js`:
- Around line 79-81: In migrate(), coerce restored sections ct, marstek, and
mqttInsights so their fields become plain objects and
marstek/mqttInsights.enabled become booleans: instead of directly spreading
...(s.ct||{}) etc., ensure you only accept object-like maps (reject arrays) for
ct.fields, marstek.fields and mqttInsights.fields and default to {} otherwise,
and coerce marstek.enabled and mqttInsights.enabled to !!value; modify the
migrate() logic (and reuse or add a small helper like isPlainObject/cleanMeter
if present) to perform these checks before constructing the returned state so
downstream code that accesses state.ct.fields / state.mqttInsights.fields can
safely assume a plain object and enabled is a boolean.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 59066a6f-eaab-45e9-91e4-3771481bd26a
📒 Files selected for processing (12)
.github/workflows/pages.yml.github/workflows/pr-preview.ymlweb/README.mdweb/index.htmlweb/js/app.jsweb/js/generate.jsweb/js/generate.test.mjsweb/js/schema.jsweb/js/schema.test.mjsweb/js/site.jsweb/js/state.jsweb/js/state.test.mjs
✅ Files skipped from review due to trivial changes (2)
- web/js/state.test.mjs
- web/js/schema.test.mjs
🚧 Files skipped from review as they are similar to previous changes (6)
- .github/workflows/pages.yml
- .github/workflows/pr-preview.yml
- web/index.html
- web/js/site.js
- web/js/app.js
- web/js/generate.js
Follow-up to the restore-sanitisation: reject non-object (incl. array) fields maps for ct/marstek/mqttInsights and coerce marstek/mqttInsights.enabled to a boolean. Adds an asObject() guard (typeof [] === 'object', so arrays are now rejected in cleanMeter's fields/tuning too). Covered by new state tests.
Apply the new palette and dark instrument-panel aesthetic across the website: - Design tokens: void/slate neutrals, energy-flow gradient (emerald -> cyan -> astra purple), Astra spark (lemon/amber/bronze), cyan interactive accent + a cyan->violet primary-button gradient; status colors retuned to the palette - Page background radial glow; hero with layered green/cyan/purple glows and an energy-arc underline; gradient step badges, how-it-works numbers, nav CTA and primary buttons; glassmorphism CTA box; recolored notes/badges/panels - Brand mark rendered as the glowing four-pointed Astra star (placeholder until the logo SVGs land); add theme-color meta CSS-only + brand glyph; no JS/behaviour changes (all tests still pass).
- web/assets/astrameter-icon.svg (gauge + Astra star mark) and astrameter-logo.svg (mark + AstraMeter wordmark), from the provided artwork - Nav + footer brand use the icon; hero features the full lockup - SVG favicon, plus PNG fallbacks (apple-touch-icon, og:image) rendered from the vectors for consistency; og:image + theme-color for link previews - Replaces the ✦ placeholder glyph
HA shows ha_addon/icon.png (square, store + header) and ha_addon/logo.png (info page). Rendered both from the vector brand assets so the add-on matches the website identity. Transparent corners + the dark plate read well on light or dark HA themes.
The <text>+system-ui wordmark relied on a font the rasteriser substituted with a wider face, so text-anchor centering overflowed to the right in the PNG exports (og:image, HA add-on logo). Outlining the wordmark to paths removes the font dependency entirely: it now renders identically and perfectly centered in every browser and renderer. Regenerated the PNG exports from the fixed SVG.
pages.yml now sets peaceiris destination_dir from the branch: main -> gh-pages root, develop -> /develop/. keep_files keeps root, /develop/, and /pr-preview/ coexisting on the branch. Docs updated; the site uses relative URLs so it works under the subpath unchanged.
Add .github/scripts/stamp-ref.sh, run by both Pages workflows before publish, to rewrite the committed @develop / blob/develop links in web/ to the ref being deployed: main -> @main, develop -> @develop, PR preview -> the PR branch. Covers the footer docs, the per-meter reference link, and the ESPHome external_components source in generated configs. Committed source stays on @develop (valid where the code lives); only the ephemeral CI copy is rewritten.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.github/workflows/pages.yml:
- Around line 49-52: Replace direct inlining of `${{ github.ref_name }}` into
the run command by setting an environment variable (e.g., DEPLOY_REF: ${{
github.ref_name }}) and update the run invocation to call the script with that
env var (e.g., run: bash .github/scripts/stamp-ref.sh "$DEPLOY_REF");
specifically modify the Pages deploy step that currently inlines github.ref_name
and instead add an env: DEPLOY_REF and use "$DEPLOY_REF" in the call so
stamp-ref.sh receives safe, data-only input rather than a pre-parsed shell
string.
In @.github/workflows/pr-preview.yml:
- Around line 43-45: The workflow currently interpolates `${{ github.head_ref
}}` directly into the inline `run:` command; treat it as untrusted input by
passing it via `env` instead and reference it as a shell variable inside the
`run` step: add an environment variable (e.g., `PR_HEAD_REF: ${{ github.head_ref
}}`) to the step and change the invocation of the script `bash
.github/scripts/stamp-ref.sh` to use the shell variable (e.g., `"$PR_HEAD_REF"`)
so the head ref is not subject to shell interpolation before the script runs.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 87da72a4-28c8-4b8a-9d15-21f70b933794
📒 Files selected for processing (4)
.github/scripts/stamp-ref.sh.github/workflows/pages.yml.github/workflows/pr-preview.ymlweb/README.md
✅ Files skipped from review due to trivial changes (1)
- .github/scripts/stamp-ref.sh
The previous stamp only covered blob/develop and @develop. Extend stamp-ref.sh to also rewrite bare repo-home links (.../astrameter -> /tree/<ref>) and README section anchors (.../astrameter#anchor -> /tree/<ref>#anchor), so the nav/hero 'GitHub' links and the install-card anchors track the deployed ref too. /issues stays ref-agnostic. Verified for plain and slashy (PR) refs.
Replace the plain-JS site + deploy-time sed (stamp-ref.sh) with a TypeScript build: - ts/ sources bundled by esbuild into dist/, type-checked by tsc, tested by tsx (package.json scripts: typecheck/test/build). Schema is now fully typed. - New ts/links.ts builds every GitHub URL (repo home, README anchors, doc files, per-meter reference, ESPHome external_components) from a single ref injected at build time via GH_REF -> __GH_REF__ (esbuild define). main->@main, develop->@develop, PR->head branch; /issues stays ref-agnostic. No post-build rewriting; remove .github/scripts/stamp-ref.sh. - Static HTML links carry data-gh attributes that site.ts resolves at load (hardcoded href kept as a no-JS fallback). - Workflows: install/typecheck/test/build, publish web/dist (main->root, develop->/develop/, PR->/pr-preview/). dist/ and node_modules gitignored. No user-visible behaviour change; tsc clean, all suites pass, built bundle verified to mount and render.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
web/ts/schema.ts (1)
659-668:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
modbusis listed as phase-capable but neither generator honors 3 phases for it.In
generate.ts,esphomeSensorhardcodesphases: 1for themodbuskind, andmeterSectionhas no per-phase handling for Modbus (nophaseListKeys, nophase: truefield). So a user toggling 3-phase on a Modbus meter sees no effect on either theconfig.inior the ESPHome YAML output. Consider removingmodbusfromPHASE_CAPABLE(or wiring per-phase register handling) so the UI toggle isn't misleading.🤖 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 `@web/ts/schema.ts` around lines 659 - 668, PHASE_CAPABLE currently includes "modbus" but the generators don't support per-phase behavior; update PHASE_CAPABLE to remove "modbus" (so the UI toggle is not misleading) or implement per-phase support: in generate.ts update esphomeSensor to use phases from meter config instead of hardcoding phases: 1 for Modbus and extend meterSection to include Modbus in phaseListKeys and/or add phase: true handling and per-phase register mappings for Modbus so the ESPHome YAML and config.ini reflect 3-phase selection; key symbols to inspect/modify: PHASE_CAPABLE, generate.ts::esphomeSensor, meterSection, phaseListKeys.web/ts/generate.ts (1)
52-67:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDownstream crash risk from unvalidated
general.
(...).join(",")(Line 55) andg.deviceIds.trim()(Line 57) assumedeviceTypesis an array anddeviceIdsa string. With untrusted restored state these can be other types and throw. The fix belongs at the sanitization boundary — see themigrate()comment inweb/ts/state.ts; no change needed here once that's addressed.🤖 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 `@web/ts/generate.ts` around lines 52 - 67, generalSection reads properties g.deviceTypes and g.deviceIds assuming types (array and string) which can crash on untrusted state; instead add validation/coercion in the state sanitization boundary (migrate() in web/ts/state.ts) so that State.general.deviceTypes is always normalized to an array (e.g., empty or single-item array) and State.general.deviceIds is coerced to a string (or undefined) before generalSection is ever called; keep generalSection unchanged but rely on migrate() to enforce these invariants for g.deviceTypes and g.deviceIds.web/ts/state.ts (1)
109-124:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
migrate()doesn't validategeneralsub-fields, defeating its stated sanitization goal for untrusted input.
general: { ...d.general, ...(s.general || {}) }shallow-merges restored values without coercing them. A crafted share link / project file can setgeneral.deviceTypesto a non-array (e.g. a string) orgeneral.deviceIdsto a non-string. That untrusted value then reachesgeneralSection()ingenerate.ts, where(...).join(",")(Line 55) andg.deviceIds.trim()(Line 57) throw aTypeError, breaking generation in the user's browser. Sincemigrate()is the designated boundary for untrusted state, coerce these fields here.🛡️ Proposed fix
const meters = Array.isArray(s.meters) && s.meters.length ? s.meters : d.meters; + const g = s.general && typeof s.general === "object" ? s.general : {}; + const general = { ...d.general, ...g }; + general.deviceTypes = Array.isArray(general.deviceTypes) + ? general.deviceTypes.filter((t: unknown): t is string => typeof t === "string") + : d.general.deviceTypes; + general.deviceIds = typeof general.deviceIds === "string" ? general.deviceIds : ""; return { ...d, ...s, target: s.target === "esphome" ? "esphome" : "python", - general: { ...d.general, ...(s.general || {}) }, + general, esphome: { ...d.esphome, ...(s.esphome || {}) },🤖 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 `@web/ts/state.ts` around lines 109 - 124, The migrate function currently shallow-merges untrusted s.general into state without coercion; update migrate (the migrate function in state.ts) to sanitize general sub-fields before returning: ensure general.deviceTypes is coerced to an array (e.g., fallback to d.general.deviceTypes when missing or not an array, and map/convert elements to strings), ensure general.deviceIds is coerced to a string (fallback to d.general.deviceIds when missing or not a string), and similarly coerce any other expected general primitives to their proper types so downstream code like generalSection() in generate.ts can safely call .join or .trim without TypeErrors; keep the merge pattern using defaultState() as baseline (d.general) but replace the direct spread of ...(s.general || {}) with a sanitized general object constructed from d.general and validated/coerced values from s.general.web/ts/app.ts (1)
425-431:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix XSS: render
state.esphome.boardwithoutinnerHTML
el()treats thehtmlprop asinnerHTML, butesphomeStepsCard()interpolatese.boardinto anhtmlstring (“Check the board matches…”).e.boardcomes from share-link/localStorage/project restore viamigrate(), which does not constrain it toESP_BOARDS, so a crafted value can inject HTML.🔒 Proposed fix: render the board as text rather than HTML
- li("<strong>Check the board matches.</strong> The file uses <code>board: " + (e.board || "esp32-s3-devkitc-1") + "</code>. This must be the board you actually bought — change it in step 2 above if not."), + el("li", {}, [ + el("strong", { text: "Check the board matches." }), + document.createTextNode(" The file uses "), + el("code", { text: "board: " + (e.board || "esp32-s3-devkitc-1") }), + document.createTextNode(". This must be the board you actually bought — change it in step 2 above if not."), + ]),🤖 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 `@web/ts/app.ts` around lines 425 - 431, The string interpolation of e.board inside the HTML passed to el()/li in esphomeStepsCard() can lead to XSS because el() treats the html prop as innerHTML; instead render the board value as text (not HTML) — e.g. stop concatenating e.board into the HTML string and inject it as a text node or escaped content (use a separate text argument/child or a dedicated text element) so state.esphome.board / e.board is inserted as plain text rather than via innerHTML.
🧹 Nitpick comments (1)
.github/workflows/pages.yml (1)
32-34: Cross-workflow contention ongh-pages.This workflow uses concurrency group
gh-pages-deploywhilepr-preview.ymlusespr-preview-${{ github.event.number }}. Since they don't share a group, a Pages deploy and a PR-preview deploy can run simultaneously and both push togh-pages, occasionally causing non-fast-forward push failures. Consider a shared concurrency group (or rely on the publishing actions' built-in retry) if you observe intermittent push errors.🤖 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/pages.yml around lines 32 - 34, The concurrency block currently sets group: gh-pages-deploy which doesn't match the PR preview workflow's pr-preview-${{ github.event.number }} group, allowing simultaneous pushes to gh-pages; change the concurrency group in this workflow to a shared static name used by both (for example "gh-pages") so Pages deploys and PR-preview deploys serialize, or alternatively coordinate both workflows to use the exact same concurrency expression; update the concurrency group value referenced in this file (group: gh-pages-deploy) to the agreed shared name to prevent cross-workflow contention.
🤖 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 `@web/README.md`:
- Around line 62-63: Update the README reference to the PR preview workflow so
it uses the full repository path for consistency: replace the standalone
"pr-preview.yml" mention with ".github/workflows/pr-preview.yml" (match the
style used for ".github/workflows/pages.yml") in the web/README.md line that
currently references "pr-preview.yml".
---
Outside diff comments:
In `@web/ts/app.ts`:
- Around line 425-431: The string interpolation of e.board inside the HTML
passed to el()/li in esphomeStepsCard() can lead to XSS because el() treats the
html prop as innerHTML; instead render the board value as text (not HTML) — e.g.
stop concatenating e.board into the HTML string and inject it as a text node or
escaped content (use a separate text argument/child or a dedicated text element)
so state.esphome.board / e.board is inserted as plain text rather than via
innerHTML.
In `@web/ts/generate.ts`:
- Around line 52-67: generalSection reads properties g.deviceTypes and
g.deviceIds assuming types (array and string) which can crash on untrusted
state; instead add validation/coercion in the state sanitization boundary
(migrate() in web/ts/state.ts) so that State.general.deviceTypes is always
normalized to an array (e.g., empty or single-item array) and
State.general.deviceIds is coerced to a string (or undefined) before
generalSection is ever called; keep generalSection unchanged but rely on
migrate() to enforce these invariants for g.deviceTypes and g.deviceIds.
In `@web/ts/schema.ts`:
- Around line 659-668: PHASE_CAPABLE currently includes "modbus" but the
generators don't support per-phase behavior; update PHASE_CAPABLE to remove
"modbus" (so the UI toggle is not misleading) or implement per-phase support: in
generate.ts update esphomeSensor to use phases from meter config instead of
hardcoding phases: 1 for Modbus and extend meterSection to include Modbus in
phaseListKeys and/or add phase: true handling and per-phase register mappings
for Modbus so the ESPHome YAML and config.ini reflect 3-phase selection; key
symbols to inspect/modify: PHASE_CAPABLE, generate.ts::esphomeSensor,
meterSection, phaseListKeys.
In `@web/ts/state.ts`:
- Around line 109-124: The migrate function currently shallow-merges untrusted
s.general into state without coercion; update migrate (the migrate function in
state.ts) to sanitize general sub-fields before returning: ensure
general.deviceTypes is coerced to an array (e.g., fallback to
d.general.deviceTypes when missing or not an array, and map/convert elements to
strings), ensure general.deviceIds is coerced to a string (fallback to
d.general.deviceIds when missing or not a string), and similarly coerce any
other expected general primitives to their proper types so downstream code like
generalSection() in generate.ts can safely call .join or .trim without
TypeErrors; keep the merge pattern using defaultState() as baseline (d.general)
but replace the direct spread of ...(s.general || {}) with a sanitized general
object constructed from d.general and validated/coerced values from s.general.
---
Nitpick comments:
In @.github/workflows/pages.yml:
- Around line 32-34: The concurrency block currently sets group: gh-pages-deploy
which doesn't match the PR preview workflow's pr-preview-${{ github.event.number
}} group, allowing simultaneous pushes to gh-pages; change the concurrency group
in this workflow to a shared static name used by both (for example "gh-pages")
so Pages deploys and PR-preview deploys serialize, or alternatively coordinate
both workflows to use the exact same concurrency expression; update the
concurrency group value referenced in this file (group: gh-pages-deploy) to the
agreed shared name to prevent cross-workflow contention.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0e71864c-d00f-4365-94ce-8c806bd3aec5
⛔ Files ignored due to path filters (1)
web/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (19)
.dockerignore.github/workflows/pages.yml.github/workflows/pr-preview.ymlweb/.gitignoreweb/README.mdweb/build.mjsweb/generator.htmlweb/index.htmlweb/package.jsonweb/ts/app.tsweb/ts/generate.test.tsweb/ts/generate.tsweb/ts/links.tsweb/ts/schema.test.tsweb/ts/schema.tsweb/ts/site.tsweb/ts/state.test.tsweb/ts/state.tsweb/tsconfig.json
💤 Files with no reviewable changes (3)
- web/ts/state.test.ts
- web/ts/schema.test.ts
- web/ts/generate.test.ts
✅ Files skipped from review due to trivial changes (2)
- web/.gitignore
- .dockerignore
🚧 Files skipped from review as they are similar to previous changes (1)
- web/generator.html
…ed CI lock - app.ts: HTML-escape the (restorable) esphome.board before it enters an innerHTML string in the ESPHome steps card (escapeHtml helper) - state.ts migrate(): coerce general sub-fields (deviceTypes->array, deviceIds & port/interval->string, flags->boolean) so generalSection can't crash on untrusted restored state; covered by new state.test assertions - schema.ts: drop 'modbus' from PHASE_CAPABLE — the generators are single-phase for Modbus, so the 3-phase toggle was misleading - pages.yml + pr-preview.yml: share one 'gh-pages' concurrency group (no cancel) so all gh-pages pushes serialise instead of racing - README: full path for .github/workflows/pr-preview.yml
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 @.github/workflows/pr-preview.yml:
- Around line 23-26: The workflow-level concurrency block (group: gh-pages /
cancel-in-progress: false) is serializing the entire PR-preview run; move this
concurrency configuration off the top-level and onto only the specific job(s)
that publish or clean up the gh-pages branch (the job that pushes/writes
gh-pages), so only those jobs use concurrency(group: gh-pages,
cancel-in-progress: false) while the rest of the workflow
(install/typecheck/test/build) runs normally.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 78c2fe9d-742d-41ec-aa0a-63aee78c5ef2
📒 Files selected for processing (7)
.github/workflows/pages.yml.github/workflows/pr-preview.ymlweb/README.mdweb/ts/app.tsweb/ts/schema.tsweb/ts/state.test.tsweb/ts/state.ts
💤 Files with no reviewable changes (1)
- web/ts/schema.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- .github/workflows/pages.yml
- web/ts/state.ts
- web/ts/state.test.ts
- web/ts/app.ts
Comment on lines
+23
to
+26
| # Shared with pages.yml so all gh-pages pushes (root / develop / pr-preview) | ||
| # serialise instead of racing. No cancel: never abort an in-flight deploy. | ||
| group: gh-pages | ||
| cancel-in-progress: false |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In GitHub Actions, does a top-level concurrencyblock serialize the entire workflow run (including build/test jobs), and what is the recommended way to serialize only the deployment job that pushes togh-pages?
💡 Result:
Yes, a top-level concurrency block in GitHub Actions serializes the entire workflow run, including all jobs within it [1][2]. When this block is defined at the workflow level, GitHub Actions ensures that only one instance of that workflow with the specified concurrency group can run at a time [1][3]. If a new run is triggered while another is in progress, the default behavior is to cancel the in-progress run [1][2]. To serialize only a specific deployment job (such as one pushing to gh-pages) without affecting the execution of your build and test jobs, you should define the concurrency block at the job level rather than the workflow level [1][2]. Recommended approach: 1. Move the concurrency block from the top level of your workflow file to the specific job responsible for deployment [1][2]. 2. Use a unique group name that represents the deployment target (e.g., gh-pages or pages-deploy). 3. Set cancel-in-progress: true if you want to ensure that only the latest deployment proceeds, or omit it/set it to false if you prefer to queue them [1][3][4]. Example configuration: jobs: build: runs-on: ubuntu-latest steps: - run: echo "Building..." deploy: needs: build runs-on: ubuntu-latest concurrency: group: gh-pages-deployment cancel-in-progress: true steps: - name: Deploy to GitHub Pages run: echo "Deploying..." By isolating the concurrency block to the deploy job, your build and test jobs can continue to run in parallel for every push, while the deployment process remains serialized to prevent conflicting writes to your deployment branch [1][2][4].
Citations:
- 1: https://docs.github.com/actions/writing-workflows/choosing-what-your-workflow-does/control-the-concurrency-of-workflows-and-jobs
- 2: https://docs.github.com/en/enterprise-cloud@latest/actions/using-jobs/using-concurrency
- 3: https://docs.github.com/en/actions/concepts/workflows-and-actions/concurrency
- 4: https://github.blog/changelog/2026-05-07-github-actions-concurrency-groups-now-allow-larger-queues/
Scope gh-pages concurrency to deploy/cleanup, not the whole workflow.
Workflow-level concurrency in .github/workflows/pr-preview.yml queues/serializes the entire PR-preview run (including npm ci/typecheck/test/build), not just the gh-pages publish portion. If the intent is only to avoid races on the gh-pages branch, move the concurrency block to the specific job that pushes/writes gh-pages (and/or the corresponding cleanup), keeping a group scoped to that deployment.
# Shared with pages.yml so all gh-pages pushes (root / develop / pr-preview)
# serialise instead of racing. No cancel: never abort an in-flight deploy.
group: gh-pages
cancel-in-progress: false🤖 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/pr-preview.yml around lines 23 - 26, The workflow-level
concurrency block (group: gh-pages / cancel-in-progress: false) is serializing
the entire PR-preview run; move this concurrency configuration off the top-level
and onto only the specific job(s) that publish or clean up the gh-pages branch
(the job that pushes/writes gh-pages), so only those jobs use concurrency(group:
gh-pages, cancel-in-progress: false) while the rest of the workflow
(install/typecheck/test/build) runs normally.
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
2 participants
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.
What this adds
A static, dummy-proof website (
web/) that walks anyone — including a complete beginner — through a few questions about their power meter and generates a ready-to-use Pythonconfig.inior ESPHome YAML, with a live preview that updates as you type. It runs entirely in the browser (nothing is uploaded).Flow (progressive disclosure — sections appear only when relevant)
config.ini) vs. ESP32 (ESPHome YAML), each explained.Every option from
config.ini.example,esphome.example.yaml, the twodocs/powermeters*.mdreferences, and the README config section is covered — general settings, per-meter tuning (throttle, smoothing, deadband, value transform, Hampel, PID), CT002/CT003 active-control tuning, multi-meterNETMASK, Marstek, and MQTT Insights.Architecture
web/js/schema.js— single data source for every meter/field/help string.web/js/generate.js— pureconfig.ini/ ESPHome YAML generators (no DOM, Node-testable).web/js/app.js— renders the form from the schema, holds state, live preview, save/load/share.web/js/generate.test.mjs— 47 assertions, run in CI before every deploy.Testing it directly from this PR
A per-PR preview workflow deploys the site to
pr-preview/pr-<number>/on thegh-pagesbranch and posts a comment here with the live URL — click it to test the real generator. The preview is torn down when the PR closes.One-time repo setup required for the URLs to resolve: Settings → Pages → Build and deployment → Source → "Deploy from a branch" →
gh-pages// (root). After that, pushes tomain/developpublish the production site and PRs get live previews.Notes
JSON_POWER_CALCULATE = Truefor signed grid power, so the generator honors each field's schema default rather than Python's blanketFalsefallback.## Nextbullet added; README gains a Configuration callout linking to the generator.https://claude.ai/code/session_01VJwSDFQexrv2DHQKoEy7Ft
Generated by Claude Code
Summary by CodeRabbit
New Features
Style
Tests
Chores