Fix/fixes wf

[atulmgupta]

Description

Closes #

Type of Change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (fix or feature that would break existing functionality)
• Documentation update
• Infrastructure / CI change

Checklist

• My code follows the project's style guidelines
• I have performed a self-review of my code
• I have added tests that prove my fix is effective or my feature works
• New and existing tests pass locally
• I have updated the documentation accordingly
• My changes generate no new warnings

Screenshots (if applicable)

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR bundles a large set of fixes spanning the Go backend, web frontend, tooling, and CI configuration. The frontend changes mostly replace hard-coded white/, black/ Tailwind utilities with CSS-variable tokens, add htmlFor/id pairing for accessibility, and switch logical RTL-aware properties (ms-*, start-*, end-*). The Go side prunes a large amount of dead code, adds doc.go files for many AI strategy packages, and broadens golangci-lint suppressions during in-flight migrations.

Changes:

• Frontend a11y/theming sweep (CSS variables, label associations, RTL logical properties, ARIA role/attribute corrections).
• Backend dead-code/unused-helper removal and addition of layer-tagging doc.go stubs across internal/ai/... and tools/....
• CI hardening (continue-on-error for image cleanup) and golangci-lint exclusion rules for SA1019/SA9003 + unused-const churn during migrations.

Reviewed changes

Copilot reviewed 179 out of 180 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
internal/api/trip_planner_handler_compute.go	Changes factor initialisation from 1.0 to zero value.
internal/api/telemetry_sessions_drive_tracking.go	Adds _ = insideAvg discards with //nolint:ineffassign to suppress warnings on intentionally-dead assignments.
internal/api/temp_impact_handler.go	Drops nil-guard for vampireDrain relying on upstream init.
internal/database/automation_history_repo.go	Removes trailing idx++ after appending Since clause.
internal/ai/provider/azure/azure.go	Removes unused ctx parameter from buildURL.
internal/ai/tools/{alert_builder,nl_grafana_panel,nl_dashboard_composer}.go	Collapses explicit struct copies into direct type conversions.
internal/api/automation_handler_decode.go	Replaces field-by-field copy with direct type conversion.
.golangci.yml	Globally excludes SA1019, SA9003, and unused-const matches under internal/ai/tools.
tools/archmetrics/baseline.json	Regenerates baseline metadata + adds many new package entries.
Many internal/ai/strategies/*/doc.go, tools/*/doc.go, internal/ml/*/doc.go	New layer-tag-only doc.go files.
Frontend *.tsx (many)	Swap white/, black/ Tailwind tokens for CSS-variable tokens.
web/src/components/layout/Layout.tsx	Migrates physical CSS utilities to logical (ms-*, start-*, end-*, inset-x-0).
web/src/components/forms/TreeSelect.tsx	Adds aria-selected on treeitems, switches to VisuallyHidden, sets tabIndex={-1} on the tree container.
web/src/components/ui/CommandPalette.tsx + test	Switches row attribute from aria-selected to aria-current.
web/src/components/status/UptimeHeatmap.tsx	Wraps each tooltip in a role="listitem" <div> and removes role="listitem" from the inner button.
web/src/components/feedback/InlineCallout.tsx + test	Removes role="status" from the anchor variant.
web/src/api/hooks/useIncidents.ts	Plumbs React Query signal through listIncidents/getIncident.
web/scripts/audit-rtl-properties.mjs	Ratchets MAX_PHYSICAL from 400 → 395.
web/scripts/audit-deferred-filter.mjs	Removes NotificationsPage from the deferred-filter target list.
.github/workflows/cleanup.yml	Adds continue-on-error: true to the package cleanup steps.
web/src/observability/rum.{ts,test.ts}	Removes a stray ESLint disable comment and an unused import.
web/src/hooks/{useRangeState,useAiStream}.ts, web/src/features/.../{MQTTInspectorPage,ChatbotPage,AlertMessageEditor,AISettings}.tsx	Removes react-hooks/exhaustive-deps disables and tightens let → const.
Multiple Go files in internal/api, internal/database, internal/automation, internal/fsm, internal/export, internal/apilog, internal/platform/httputil	Deletes unused helpers, fields, and imports.
docs/index.md	Strips the /teslasync prefix from image paths.

Comments suppressed due to low confidence (7)

internal/api/trip_planner_handler_compute.go:1

• Changing the initial value of factor from 1.0 to the zero value (var factor float64 → 0) is a behavior change. factor is the multiplicative weather adjustment applied to range/energy estimates; a baseline of 1.0 means "no impact", while 0 means "no range at all". Unless every code path below this point unconditionally assigns factor before it is consumed, this will silently zero out the estimate for any temperature that falls outside the explicit adjustment buckets (e.g. mild temperatures with no penalty). Please restore factor := 1.0 or verify with a test that every branch assigns it.
  internal/api/telemetry_sessions_drive_tracking.go:1
• Assigning to insideAvg and immediately discarding it with _ = insideAvg (plus a //nolint:ineffassign,staticcheck waiver) is hard to reason about — readers will assume the value is consumed downstream. If the field really is dead until the persistence rewrite lands, prefer either (a) deleting both the computation and the variable entirely and leaving the comment as a TODO, or (b) keeping the field write but removing the _ = insideAvg no-op so the suppression alone documents intent. The same pattern is repeated a few lines below for the startSnapshot fallback.
  internal/database/automation_history_repo.go:1
• Removing the trailing idx++ is safe today because the Since clause is the last placeholder-allocating branch in buildHistoryWhere, but it makes the function fragile: any future clause inserted after this one will silently reuse $idx and bind to the wrong argument. Consider keeping the idx++ so the invariant "after appending a $N clause, idx always points at the next unused placeholder" holds uniformly across all branches.
  internal/api/temp_impact_handler.go:1
• The comment claims vampireDrain is initialised to a non-nil empty slice earlier in the handler, but this PR only removes the guard — please double-check that the upstream initialisation actually exists on every code path (including early-return / error / cache-hit branches). If any path can leave vampireDrain == nil, dropping the nil-guard will cause the JSON response to emit null instead of [], which is a breaking shape change for the frontend.
  web/src/components/forms/TreeSelect.tsx:1
• On role="treeitem", aria-selected and aria-checked represent two different states and should not both be driven from the same boolean. With aria-multiselectable="true" on the parent (line 459), aria-selected is the canonical "is this item in the selection set" signal; aria-checked="mixed" additionally communicates partial selection for the group node. Setting aria-selected={allGroupSelected} for a group with someGroupSelected === true will report the group as "not selected" while aria-checked is "mixed", which screen readers may announce inconsistently. Consider using aria-selected={allGroupSelected || someGroupSelected} for the group node, or dropping aria-selected on group rows and relying solely on aria-checked for the tri-state.
  web/src/components/ui/CommandPalette.tsx:1
• aria-current is defined to take a token value (page, step, location, date, time, true, false) and is semantically "represents the current item within a set" — typically used for navigation/breadcrumbs, not for "the currently highlighted row in a listbox-style picker". The previous aria-selected was a better fit if you switch the container to role="listbox" and each row to role="option". As-is, screen readers in a command-palette context will receive a less-conventional announcement. Consider either restoring aria-selected along with the correct container/child roles, or using aria-current="true" explicitly (the boolean coercion to the string "true" works today but is undocumented).
  internal/api/automation_handler_decode.go:1
• Replacing the explicit field-by-field copy with a direct automationInputWire(def) conversion only compiles while the two struct types stay field-identical (same order, same names, same types, same tags). The previous explicit form would still compile and produce a clear diff if either struct grew or reordered a field; the new form will silently break or refuse to compile with a less obvious error. If you keep the conversion, add a compile-time assertion (e.g. var _ = automationInputWire(automationPortable{}) in an init block, or a //nolint comment documenting the invariant) so the coupling is explicit.

[Copilot]

Pull request overview

Copilot reviewed 188 out of 190 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (9)

internal/api/trip_planner_handler_compute.go:1

• Changing the initial value of factor from 1.0 to the zero value (0.0) silently alters semantics: any code path through this function that does not subsequently assign factor will now apply a 0× weather multiplier instead of a 1× no-impact multiplier (likely zeroing out range/energy estimates). If the intent was to detect "no rule matched" via a sentinel, use an explicit var factor = 1.0 (the previous behaviour) or guard the multiplication with an assigned boolean — but please don't ship a default of 0.
  internal/api/telemetry_sessions_drive_tracking.go:1
• Adding _ = insideAvg + a per-site //nolint comment to keep a write that nothing reads is hard to justify — the linter is telling the truth, and the // dead post-mig 000185 rationale is exactly what a deletion comment should say. Please either (a) delete the dead computation and leave the explanatory comment, or (b) leave a single TODO referencing the persistence-rewrite ticket and remove the assignment. Sprinkling _ = x to silence the linter trains readers to ignore future ineffassign hits.
  internal/database/automation_history_repo.go:1
• Dropping idx++ here is only safe if the Since clause is guaranteed to be the last placeholder-using clause in buildHistoryWhere. If any future caller adds another conditional clause below this one, the new clause will reuse the same $%d index that Since consumed. Consider either keeping the idx++ for future-proofing or adding a comment explicitly noting that this must remain the terminal clause.
  web/src/components/forms/TreeSelect.tsx:1
• For a group node with a tri-state checkbox child (aria-checked="mixed" when partially selected), setting aria-selected={false} on the treeitem while the partial state is visible can be confusing to screen readers — selected and checked convey overlapping but distinct meanings. Consider only emitting aria-selected for the fully-selected case and omitting it (or setting undefined) otherwise so AT does not announce "not selected" for partial groups.
  web/src/components/status/UptimeHeatmap.tsx:1
• Moving role="listitem" from the interactive <button> to a wrapping non-interactive <div> works for the list semantics, but the button itself no longer has any inherent grouping role. If the parent container relies on role="list", ensure the <div role="listitem"> is a direct child of that list element — interposing the <Tooltip> between them may break the list ↔ listitem pairing in some screen readers.
  web/vite.config.ts:1
• The CI workflow's frontend coverage step reads web/coverage/coverage-summary.json — that file is only produced when the json-summary reporter is registered and --coverage runs. The new CI step uses if: always() && hashFiles(...) so it'll silently skip if the file is missing; consider failing the job (or at least emitting a warning to $GITHUB_STEP_SUMMARY) when coverage was expected but not produced, so a future reporter-list edit doesn't quietly stop publishing the summary.
  web/src/test-setup.ts:1
• The inline defaults object duplicates the defaults: AppSettings literal inside the real hook. Because there is no shared source of truth, drift between this file and the production defaults will silently change test behaviour without any test failure. Consider exporting the production defaults from useSettings.ts (e.g. export const DEFAULT_APP_SETTINGS) and importing it here so the two stay locked.
  web/src/tests/lazyRoutes.list.ts:1
• These "redirect" components likely use a synchronous <Navigate> element — wrapping them with React.lazy() via import(...) adds a network roundtrip (chunk download) just to perform a client-side redirect. Consider importing the redirects eagerly so users hitting legacy URLs do not see a brief Suspense fallback before navigation.
  .github/workflows/ci.yml:1
• The PR title ("Fix/fixes wf") and description are placeholder/empty — none of the checkboxes are ticked and no issue is linked, but the diff touches CI, lint config, dozens of UI components, backend handlers, AI strategy packages, and behavioural code (e.g. trip_planner_handler_compute.go). Please populate the description with the scope of the change and the rationale for the behavioural edits so reviewers can verify intent.

name: CI