Skip to content

chore(ci): lint translation files (invalid JSON, dup keys, en/de parity)#277

Open
TheDave94 wants to merge 4 commits into
TheRealSimon42:mainfrom
TheDave94:chore/translation-lint-ci
Open

chore(ci): lint translation files (invalid JSON, dup keys, en/de parity)#277
TheDave94 wants to merge 4 commits into
TheRealSimon42:mainfrom
TheDave94:chore/translation-lint-ci

Conversation

@TheDave94
Copy link
Copy Markdown

Summary

Adds a translation-file linter that catches three classes of bug `JSON.parse` silently swallows:

  1. Invalid JSON — covered by JSON.parse.
  2. Duplicate keys — JSON.parse keeps only the last value, dropping the earlier ones without raising. The linter re-tokenizes the raw text so it sees every key occurrence and reports each duplicate with line number.
  3. Key-parity drift between en.json and de.json — a key in one locale but not the other means users on the missing locale will see the raw `localize()` identifier instead of translated text.

Wired into `.github/workflows/validate.yml` as a separate `Translation Lint` job (Node 20).

Why now

PR #276 in this repo failed Codacy yesterday on exactly issue (2) — a duplicate `show_search_card_missing` key introduced during a merge. JSON.parse accepted it; the duplicate would have shipped silently into the bundle if Codacy hadn't caught it. This step catches the same class of bug in CI, deterministically, in seconds, regardless of whether Codacy is enabled.

Test plan

  • Workflow runs on push/PR
  • On main as-is (clean state) → passes
  • Inject a duplicate key locally → linter reports it with line number
  • Remove a key from de.json that exists in en.json → linter reports parity drift

AI-drafted under human supervision by @TheDave94. Tested live on Home Assistant — fully when the relevant hardware is available, otherwise only along the code paths that don't require an actual sensor of that type.

…arity

Adds scripts/lint-translations.mjs and wires it into the Validate workflow
as a separate "Translation Lint" job. Catches three classes of bug that
JSON.parse silently swallows:

1. Invalid JSON (covered by JSON.parse itself).
2. Duplicate keys — JSON.parse keeps only the last value, so duplicates
   silently lose information. The script re-tokenizes the raw text to see
   every key occurrence.
3. Key-parity drift between en.json and de.json — a key in one but not
   the other means one of the two locales will fall through to the raw
   localize() identifier in the UI.

Self-test: linter is exercised against a known-good fixture in CI; an
inline test during development confirmed it catches injected duplicates
on the right line number.
@codacy-production
Copy link
Copy Markdown

codacy-production Bot commented May 19, 2026

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 35 complexity

Metric Results
Complexity 35

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

TheDave94 (via AI) added 2 commits May 19, 2026 10:53
Two stubborn warnings the file-level eslint-disable didn't fully suppress:

- ESLint security/detect-non-literal-fs-filename: readFileSync(f, ...)
  with a loop variable triggers the non-literal-path warning. Fixed by
  threading reads through a readTranslation() helper that dispatches
  on equality to one of two literal constants (EN_PATH, DE_PATH); any
  other path throws. The fs calls now use literal arguments only.

- Biome useQwikValidLexicalScope: a stray Qwik-framework rule applied
  to a plain Node script flagged the arrow-function `report = (...) => ...`
  binding. Replaced with a function declaration which is outside the
  rule's scope.

Behavior unchanged — same three checks (JSON validity, duplicate keys,
en/de key parity), same exit codes. Self-test against a synthetic
duplicate still fires correctly.
TheDave94 pushed a commit to TheDave94/oriel-dashboard that referenced this pull request May 20, 2026
Brings into main the remaining open PRs that weren't part of the prior
consolidation (bb43b04):

Improvement PRs (8 — real content):
- TheRealSimon42#277 chore(ci): translation lint guard
- TheRealSimon42#278 test: section-builder + entity-filter unit tests + snapshots
- TheRealSimon42#279 feat(rooms): auto-detect humidifier/valve/water_heater
- TheRealSimon42#280 feat(editor): derive target_section dropdown from section meta
- TheRealSimon42#281 fix(areas): auto-hide section + audit test
- TheRealSimon42#282 feat(editor): wire show_window_contacts_in_rooms + show_door_contacts_in_rooms
- TheRealSimon42#283 feat: custom_sections — user-declared sections without forking
- TheRealSimon42#284 chore(ci): release-please + release-build + ESLint enforcement

Grouped PRs (7 — content already in main via bb43b04 consolidation;
recorded with `-s ours` so the merges land in history without
duplicate/regressive edits):
- TheRealSimon42#270 grouped/optional-overview-sections
- TheRealSimon42#271 grouped/live-overview-badges
- TheRealSimon42#272 grouped/battery-view-improvements
- TheRealSimon42#273 grouped/room-view-features
- TheRealSimon42#274 grouped/section-meta-security
- TheRealSimon42#275 grouped/covers-weather
- TheRealSimon42#276 grouped/persons-overview-tweaks

CI fixes uncovered during merge:
- Bump Node 20 → 22 in validate.yml + release-build.yml (ESLint 10 requires Node 22+)
- Load eslint-plugin-security (rules off) so source-level Codacy
  disable directives resolve cleanly
- Drop unused PropertyValues import in StrategyEditor.ts

Version 1.3.4-thedave-r2 → 1.4.0-thedave (minor bump: feat: custom_sections,
auto-detect humidifier/valve, editor coverage, release automation).

Dist rebuilt + tests + ESLint pass (0 errors, 0 warnings).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@TheDave94 TheDave94 closed this May 20, 2026
@TheDave94 TheDave94 deleted the chore/translation-lint-ci branch May 20, 2026 22:44
@TheDave94 TheDave94 restored the chore/translation-lint-ci branch May 21, 2026 08:56
@TheDave94 TheDave94 reopened this May 21, 2026
@TheDave94
Copy link
Copy Markdown
Author

TheDave94 commented May 21, 2026

My main focus has shifted to a downstream project (Oriel Dashboard) — built on what simon42 established, taken in its own direction — but the work in this PR was always meant for simon42 and I'm happy to keep iterating on it here. Address review feedback, split bundled changes into smaller pieces, whatever helps the review. Just ping me.

TheDave94 pushed a commit to TheDave94/oriel-dashboard that referenced this pull request May 21, 2026
Brings into main the remaining open PRs that weren't part of the prior
consolidation (ee9c67c):

Improvement PRs (8 — real content):
- TheRealSimon42#277 chore(ci): translation lint guard
- TheRealSimon42#278 test: section-builder + entity-filter unit tests + snapshots
- TheRealSimon42#279 feat(rooms): auto-detect humidifier/valve/water_heater
- TheRealSimon42#280 feat(editor): derive target_section dropdown from section meta
- TheRealSimon42#281 fix(areas): auto-hide section + audit test
- TheRealSimon42#282 feat(editor): wire show_window_contacts_in_rooms + show_door_contacts_in_rooms
- TheRealSimon42#283 feat: custom_sections — user-declared sections without forking
- TheRealSimon42#284 chore(ci): release-please + release-build + ESLint enforcement

Grouped PRs (7 — content already in main via ee9c67c consolidation;
recorded with `-s ours` so the merges land in history without
duplicate/regressive edits):
- TheRealSimon42#270 grouped/optional-overview-sections
- TheRealSimon42#271 grouped/live-overview-badges
- TheRealSimon42#272 grouped/battery-view-improvements
- TheRealSimon42#273 grouped/room-view-features
- TheRealSimon42#274 grouped/section-meta-security
- TheRealSimon42#275 grouped/covers-weather
- TheRealSimon42#276 grouped/persons-overview-tweaks

CI fixes uncovered during merge:
- Bump Node 20 → 22 in validate.yml + release-build.yml (ESLint 10 requires Node 22+)
- Load eslint-plugin-security (rules off) so source-level Codacy
  disable directives resolve cleanly
- Drop unused PropertyValues import in StrategyEditor.ts

Version 1.3.4-thedave-r2 → 1.4.0-thedave (minor bump: feat: custom_sections,
auto-detect humidifier/valve, editor coverage, release automation).

Dist rebuilt + tests + ESLint pass (0 errors, 0 warnings).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@TheDave94 TheDave94 closed this May 22, 2026
@TheDave94 TheDave94 deleted the chore/translation-lint-ci branch May 22, 2026 05:55
@TheDave94 TheDave94 restored the chore/translation-lint-ci branch May 23, 2026 20:44
@TheDave94 TheDave94 reopened this May 23, 2026
…ns.mjs

Codacy flagged 7 high-severity "Generic Object Injection Sink"
warnings on this script — all from bracket access on dynamic
indices (text[i] in findDuplicateKeys, obj[k] in collectKeys).
The prior fix used a file-level eslint-disable comment, which the
in-repo ESLint run honors but Codacy's separate scanner does not.

Eliminate the warnings at the source instead of suppressing them:

  - text[i] -> text.charAt(i) at all 6 sites in findDuplicateKeys.
    Identical behavior for in-range indices; the loop's `i < len`
    guard ensures we never read out-of-range. Codacy sees a method
    call instead of a bracket access and the rule no longer fires.

  - Object.keys(obj) + obj[k] -> Object.entries(obj) destructured
    to [k, v] in collectKeys. Same shape, no bracket lookup.

The file-level eslint-disable comment is now redundant and removed.
Two structural comments at the changed sites explain the why for
future readers.

Verified locally:
  - `node scripts/lint-translations.mjs` against en/de.json -> OK,
    exit 0.
  - Synthetic duplicate injected into en.json still caught with the
    correct line number; en/de parity check still flags the
    now-missing-in-de key.
TheDave94 added a commit to TheDave94/oriel-dashboard that referenced this pull request May 23, 2026
…n extension procedure (#62)

* chore(lint-translations): apply Codacy-clean patterns from simon42 TheRealSimon42#277

Same fix shape that landed upstream in simon42 PR TheRealSimon42#277, applied
preemptively here. Codacy isn't currently active on this repo, but
ESLint's file-level disable is the wrong fix in two ways: it hides
the pattern from any future scanner, and the pattern has now been
cargo-cult-copied into a Codacy-active repo once.

  - text[i] -> text.charAt(i) at all 9 sites in findDuplicateKeys.
    Identical behavior for in-range indices; the loop's `i < len`
    guard ensures we never read out-of-range.

  - Object.keys(obj) + obj[k] -> Object.entries(obj) destructured
    to [k, v] in collectKeys AND in the additional checkHtmlSafety
    site (which doesn't exist in the simon42 version).

  - parsed[file] -> new Map() with .set / .get throughout the
    locale-discovery loop. Cleaner type for a string-keyed lookup
    table too; not just scanner appeasement.

The `detect-object-injection` half of the file-level eslint-disable
is now redundant and removed. The `detect-non-literal-fs-filename`
half stays — it covers `readFileSync(file, ...)` where `file` is a
loop variable over the readdir result, which the simon42 fix
handled differently (two-literal-paths dispatch) but doesn't scale
to the N-locale model here.

Verified locally:
  - `node scripts/lint-translations.mjs` against en/de -> OK
  - Synthetic duplicate injected into en.json -> caught with the
    correct line number and en/de parity check still fires.
  - `npm test tests/scripts/lint-translations.test.ts` -> 7 pass.

* docs(protected-branches): how to extend protection to additional upstreams

The snapshot file and the L2 CI guard are both hard-coded to
TheRealSimon42/simon42-dashboard-strategy. If a future PR from
this repo targets a different upstream, neither covers it — same
silent-cascade hazard as the simon42 incident, just pointed at a
different repo.

Adds an "Extending protection to additional upstreams" section
before the existing "When to refresh" block, with the four-step
procedure: update the snapshot query, update the workflow query,
refresh this file, update CLAUDE.md's Cross-fork PR safety section.
Closes with an explicit fail-open warning — a new upstream is not
covered until all four steps land.

---------

Co-authored-by: AutoCoder <autocoder@claudebox>
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.

1 participant