refactor: Extract Ali je vroče read model#418
Conversation
There was a problem hiding this comment.
Pull request overview
This PR extracts the “Ali je vroče” station/current-hotness read model into a plain TypeScript module (with tests) while keeping the existing useWeatherData() UI-facing shape, and wires yarn test + yarn typecheck into CI.
Changes:
- Introduces
code/ali-je-vroce/model/readModel.tsto own preference storage/migration, station shape conversion, current-hotness display mapping, optimistic display policy, and raw response transforms. - Updates
useWeatherData+helpers.tsto use the read model and removes the old optimistic-update utility. - Adds Node test coverage for the read model and runs
yarn test+yarn typecheckin the build workflow.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Adjusts TS path-mapping configuration comment / baseUrl usage. |
| skills-lock.json | Adds skills lock scaffold for agent-skill management. |
| README.md | Documents canonical vs generated agent skills locations and sync command. |
| package.json | Adds sync-skills, updates postinstall, adds test and typecheck scripts. |
| code/ali-je-vroce/utils/optimistic.ts | Removes legacy optimistic-update helper (now owned by read model). |
| code/ali-je-vroce/model/readModel.ts | New read-model module (prefs, transformations, optimistic display). |
| code/ali-je-vroce/model/readModel.test.ts | New Node tests for the read-model behavior. |
| code/ali-je-vroce/hooks/useWeatherData.ts | Switches UI hook to read-model state/transformations + new optimistic policy. |
| code/ali-je-vroce/helpers.ts | Delegates station/current-hotness transforms to the read model. |
| CLAUDE.md | Points Claude Code at the canonical agent instructions. |
| AGENTS.md | Adds canonical, tool-neutral agent instruction entrypoint and skills guidance. |
| .gitignore | Ignores generated .claude/skills copy. |
| .github/workflows/copilot-setup-steps.yml | Adds Copilot coding-agent setup workflow for consistent environment. |
| .github/workflows/build.yaml | Runs yarn test and yarn typecheck in CI before yarn build. |
| .github/instructions/typescript-solid-eleventy.instructions.md | Adds scoped frontend/TS/Solid/Eleventy guidance. |
| .github/instructions/testing.instructions.md | Adds scoped testing/validation guidance. |
| .github/instructions/security.instructions.md | Adds scoped security guidance for code/workflows/content/data. |
| .github/instructions/performance.instructions.md | Adds scoped performance guidance for site + frontend. |
| .github/instructions/documentation.instructions.md | Adds scoped docs/content guidance. |
| .github/instructions/code-review.instructions.md | Adds scoped code review guidance for this repo. |
| .github/copilot-instructions.md | Adds canonical repository-wide Copilot instructions. |
| .github/agents/software-engineer.agent.md | Adds “Software Engineer” agent definition. |
| .github/agents/reviewer.agent.md | Adds “Reviewer” agent definition. |
| .github/agents/debugger.agent.md | Adds “Debugger” agent definition. |
| .github/agents/architect.agent.md | Adds “Architect” agent definition. |
| .agents/skills/write-tests/SKILL.md | Adds reusable “write-tests” skill. |
| .agents/skills/setup-component/SKILL.md | Adds reusable “setup-component” skill. |
| .agents/skills/refactor-code/SKILL.md | Adds reusable “refactor-code” skill. |
| .agents/skills/generate-docs/SKILL.md | Adds reusable “generate-docs” skill. |
| .agents/skills/debug-issue/SKILL.md | Adds reusable “debug-issue” skill. |
| .agents/skills/code-review/SKILL.md | Adds reusable “code-review” skill. |
| const initialState = createInitialReadModelState(localStorage); | ||
| const [stationId, setStationId] = createSignal(String(initialState.selectedStation?.value)); | ||
| const [state, setState] = createStore<ReadModelState>(initialState); |
There was a problem hiding this comment.
Accepted. The read model catches getItem/setItem/removeItem failures, but this comment catches the missing outer edge: evaluating localStorage itself can throw before the read-model fallback runs. Implemented getBrowserPreferenceStorage() in the Solid hook, which falls back to a no-op preference storage adapter when browser storage is unavailable. Covered by existing storage-failure read-model tests plus yarn test / yarn typecheck after the change. Commit: 49103f9.
|
|
||
| // Save selected station to local storage (in cached format) | ||
| setPreference('selectedStation', cachedFormat); | ||
| writeSelectedStationPreference(localStorage, selectedStation); |
There was a problem hiding this comment.
Accepted. Same underlying issue as the initialization path: passing localStorage directly means the getter can throw before writeSelectedStationPreference has a chance to handle storage errors. The hook now acquires preferenceStorage once through the safe wrapper and uses that same adapter for writes during station changes. Commit: 49103f9.
Stacking note
This PR is stacked on #416 and should be reviewed against
chore/prepare-repo-for-agents, not directly againstmain.That matters because the new frontend-check workflow runs
yarn typecheck, which depends on the TypeScript configuration fix from #416. The unrelatedchore/upgrade-yarn-depsbranch is no longer part of this branch chain.Why
The
Ali je vročeflow had several read-model responsibilities spread across the Solid hook, query helpers, network helpers, and an optimistic-update utility. Understanding station selection required following localStorage preference migration, station shape conversion, current-hotness display mapping, raw Datasette/Vremenar transformations, cache lookup behavior, and optimistic placeholders across multiple modules.This PR deepens that module boundary by putting the station/current-hotness read model behind a plain TypeScript interface while preserving the existing UI-facing
useWeatherData()return shape.What changed
code/ali-je-vroce/model/readModel.tsas a plain read-model module.yarn testand wiredyarn testplusyarn typecheckinto the website build workflow.Impact
The page behavior is intended to stay the same, with a few deliberate improvements:
localStoragedoes not break page initialization or station switching.Follow-up
Created #417 for the next related slice: reducing duplicated query/cache fallback policy across
hooks/queries.ts,utils/prefetching.ts, andhooks/useWeatherData.ts.Validation
yarn testyarn typecheckyarn build