Skip to content

refactor(web): adopt <DataState> in fizruk Workouts journal#1709

Merged
Skords-01 merged 1 commit into
mainfrom
devin/1777905144-fizruk-datastate-adoption
May 4, 2026
Merged

refactor(web): adopt <DataState> in fizruk Workouts journal#1709
Skords-01 merged 1 commit into
mainfrom
devin/1777905144-fizruk-datastate-adoption

Conversation

@Skords-01
Copy link
Copy Markdown
Owner

@Skords-01 Skords-01 commented May 4, 2026

Summary

Адаптуємо <DataState> wrapper у fizruk/pages/Workouts.tsx — другий споживач (після PR 2.4) присутньої але до цього часу не використаної інфраструктури з PR 2.2. Замінюємо ad-hoc патерн view === "log" && !workoutsLoaded ? <Skeleton /> : <WorkoutJournalSection /> на <DataState> з контрактом «data === undefined → skeleton; data визначено → content (навіть під час cloud-pull stale-revalidate)».

  • Workouts (apps/web/src/modules/fizruk/pages/Workouts.tsx): єдина точка з Skeleton-based loading state у fizruk модулі. useWorkouts flip-ає loaded з false → true після першого tick гідрації (з localStorage / SQLite). DataState query: data: workoutsLoaded ? workouts : undefined, isLoading: !workoutsLoaded. Це дозволяє WorkoutJournalSection рендерити свою власну «порожньо» empty-state без миготіння під час mount.
  • Skeleton-розмітка залишається ідентичною (3 row-shaped Skeleton) — лише огорнута DataState skeleton slot-ом.

Поведінка-зберігаюча міграція. Hooks/business logic не змінюються — лише presentation.

Інші fizruk-сторінки (Dashboard, Programs, Atlas, Body/*, Progress, Exercise) не мають Skeleton-based loading state — вони або працюють синхронно з local-first MMKV-web даними, або вже degrade-ять до empty defaults. Тому скоуп цього PR — рівно один файл (на відміну від 2.4, який торкав три).

Governing Skill

  • Primary skill: sergeant-web-ui
  • Secondary skill (if truly needed): sergeant-feature-delivery

Playbook

  • Primary playbook: docs/playbooks/web-feature-shipping.md
  • Why this playbook: presentational web рефактор з типчеком, ESLint, vitest. Той самий patternі формат, що й 2.4 (refactor(web): adopt <DataState> in finyk Mono panels #1703).
  • If no playbook matched, why: n/a

Verification

pnpm --filter @sergeant/web typecheck
pnpm exec eslint src/modules/fizruk/pages/Workouts.tsx
pnpm --filter @sergeant/web exec vitest run src/modules/fizruk

Результати локально:

  • typecheck — pass.
  • eslint (точково на Workouts.tsx) — pass без warnings.
  • vitest (src/modules/fizruk) — 15 files / 93 tests pass. Жодних регресій у workout-related тестах (useWorkouts.test.tsx, WorkoutJournalSection.finish.test.tsx, тощо).

Без нового RTL-теста для Workouts.tsx: компонент композитує 8+ хуків (useExerciseCatalog, useRecovery, useWorkouts, useMonthlyPlan, useWorkoutTemplates, useToast, useFizrukRestSound, useActiveFizrukWorkout) і переносний integration test потребував би мокати їх усі. DataState routing логіка вже покрита 11 тестами в apps/web/src/shared/components/ui/DataState.test.tsx, а існуючі useWorkouts.test.tsx (4 tests) фіксують контракт loaded flag. Якщо рев'юер вважає за потрібне — можемо додати окремим follow-up.

Additional checks:

  • Local smoke / manual validation completed (typecheck + eslint + vitest fizruk above)
  • Surface-specific checks completed (DataState contract: data === undefined triggers skeleton; persisted data + isLoading=true тримає content і не блимає у skeleton)

Docs and Governance

  • I updated docs that changed with the behavior, contract, workflow, or rollout.
  • I checked whether AGENTS.md needed an update.
  • I checked whether a playbook or skill needed an update.
  • I checked whether governance docs or review docs needed an update.

Updated docs:

  • docs/initiatives/0011-foundation-adoption-and-process-discipline.md буде оновлено в окремому follow-up commit / PR після merge обох PR-ів (2.4 refactor(web): adopt <DataState> in finyk Mono panels #1703 і 2.5) — там відмітимо їх як done з лінками. AGENTS.md / playbooks / governance / review docs без змін — <DataState> API вже задокументовано в apps/web/src/shared/components/ui/DataState.tsx JSDoc + DataState.stories.tsx.

Risk and Rollout

  • User-visible risk: низький. Behavior-preserving presentational refactor — той самий skeleton, той самий журнал тренувань. Stale-revalidate (cloud pull → re-merge) тепер не повертає skeleton (раніше workoutsLoaded залишалося true після гідрації, тож практично ніколи не повертало skeleton; цей invariant збережено).
  • Rollout / deploy order: звичайний deploy через Vercel preview → main. Без міграцій, без env-змін, без feature-flag.
  • Backout plan: revert PR; жодних persisted side effects (localStorage / MMKV / RQ keys / API contracts) не зачеплено.

Hard Rule #15

  • I read AGENTS.md before coding.
  • Internal docs I touched are in Ukrainian.
  • I did not use --no-verify.

Reviewer Notes

  • DataState contract зведено через data: workoutsLoaded ? workouts : undefined. Семантика — точно та сама, що й попередній view === "log" && workoutsLoaded guard, але виражена через єдину абстракцію.
  • useWorkouts.loaded гарантовано flip-ається в true після useEffect mount-а (підтверджено useWorkouts.test.tsx line 30 — await waitFor(() => expect(result.current.loaded).toBe(true))). Тож DataState skeleton slot бачимо лише на першому tick після mount.
  • Залишок Workouts JSX (view === "home" / "catalog" / "templates", WorkoutsHome, WorkoutCatalogSection, WorkoutTemplatesSection) не має loading state і свідомо лишений поза DataState — це не RQ-driven UI.
  • Pair-PR 2.4 (refactor(web): adopt <DataState> in finyk Mono panels) — refactor(web): adopt <DataState> in finyk Mono panels #1703.

Summary by cubic

Adopted <DataState> in the Fizruk Workouts journal to centralize loading and prevent the empty-state flash during hydration. Replaces the ad‑hoc view === "log" skeleton guard without changing behavior.

  • Refactors
    • Uses <DataState> from @shared/components/ui/DataState with: data === undefined → skeleton; defined data → content (even when isLoading is true).
    • Maps query as data: workoutsLoaded ? workouts : undefined and isLoading: !workoutsLoaded to show the list across stale‑revalidate.
    • Keeps the same 3‑row skeleton, now in the skeleton slot.
    • No hooks or business logic changed; scope is apps/web/src/modules/fizruk/pages/Workouts.tsx only.

Written for commit 1d69985. Summary will update on new commits.

Summary by CodeRabbit

  • Refactor
    • Improved loading state handling for the workout journal with enhanced skeleton display during initial data loading.

Replace the `view === 'log' && !workoutsLoaded` skeleton guard with a
unified <DataState> wrapper that owns the skeleton/content slot routing
for the workout journal.

- The `useWorkouts` hook flips `loaded` from false -> true after the
  first hydration tick (rehydrating from localStorage / SQLite). While
  `loaded` is false the DataState query feeds `data: undefined` and the
  skeleton slot renders; from the second tick on `data` is the real
  workouts list (possibly empty), so the journal can render its own
  empty state without flashing it during hydration.
- `isLoading` mirrors the inverted `loaded` flag so a future
  stale-revalidate (cloud pull -> re-merge) keeps the journal visible
  rather than collapsing back to the skeleton.

Behavior-preserving migration. Hooks/business logic untouched;
presentational only. fizruk module has exactly one Skeleton-based
loading site (this one) -- no other panels needed migration in this PR.

Refs: docs/initiatives/0011-foundation-adoption-and-process-discipline.md
(Phase 2, PR 2.5)
@vercel
Copy link
Copy Markdown
Contributor

vercel Bot commented May 4, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
sergeant Ready Ready Preview, Comment May 4, 2026 2:41pm

Request Review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 4, 2026

📝 Walkthrough

Walkthrough

The Workouts component refactors its workout journal rendering to use a unified DataState wrapper instead of separate conditional blocks for loading and loaded states. A journalQuery object is introduced to standardize the loading state contract, replacing manual workoutsLoaded checks with an explicit skeleton component.

Changes

Workout Journal Rendering Pattern

Layer / File(s) Summary
Imports & Type Setup
apps/web/src/modules/fizruk/pages/Workouts.tsx
Adds DataState and type DataStateQueryLike imports to support the new rendering pattern.
State Object Definition
apps/web/src/modules/fizruk/pages/Workouts.tsx
Defines journalQuery object with data (undefined while loading, workouts when loaded) and isLoading flag, and creates workoutsLoadingSkeleton JSX for the loading state.
Conditional Rendering Replacement
apps/web/src/modules/fizruk/pages/Workouts.tsx
Replaces separate conditional blocks for loading and loaded states with a single DataState wrapper that manages both the skeleton display and WorkoutJournalSection render.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

size/L

Poem

🐰 A skeleton dances, then workouts appear,
DataState wraps them with pattern so clear,
No more scattered conditions to hide,
Just one unified wrapper with grace as our guide! 💪

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: adopting the DataState wrapper in the fizruk Workouts journal component.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch devin/1777905144-fizruk-datastate-adoption

Review rate limit: 0/10 reviews remaining, refill in 59 minutes and 48 seconds.

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/web/src/modules/fizruk/pages/Workouts.tsx`:
- Around line 554-555: DataState is treating an empty workouts array as
empty-state and skipping WorkoutJournalSection; update the DataState usage
(which currently receives journalQuery) to only treat null/undefined as empty by
supplying a custom emptiness checker so an empty array still renders the journal
UI. Locate DataState with journalQuery and add an isEmpty (or equivalent prop
supported by DataState) that returns true only when data == null/undefined
(e.g., isEmpty: data => data == null) so WorkoutJournalSection sees an empty
workouts array and can render its own empty UI.
🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d4612d95-f6fd-4c02-9040-db57d05feba1

📥 Commits

Reviewing files that changed from the base of the PR and between fbd4e36 and 1d69985.

📒 Files selected for processing (1)
  • apps/web/src/modules/fizruk/pages/Workouts.tsx

Comment on lines +554 to +555
<DataState query={journalQuery} skeleton={workoutsLoadingSkeleton}>
{() => (
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

DataState will treat empty workouts arrays as empty-state and skip WorkoutJournalSection.

With current props, once loading finishes and workouts is [], DataState default emptiness handling can render its own empty slot instead of the journal’s internal empty UI. That regresses the intended behavior described in this PR.

Suggested fix
-          <DataState query={journalQuery} skeleton={workoutsLoadingSkeleton}>
+          <DataState
+            query={journalQuery}
+            skeleton={workoutsLoadingSkeleton}
+            isEmpty={() => false}
+          >
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<DataState query={journalQuery} skeleton={workoutsLoadingSkeleton}>
{() => (
<DataState
query={journalQuery}
skeleton={workoutsLoadingSkeleton}
isEmpty={() => false}
>
{() => (
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/src/modules/fizruk/pages/Workouts.tsx` around lines 554 - 555,
DataState is treating an empty workouts array as empty-state and skipping
WorkoutJournalSection; update the DataState usage (which currently receives
journalQuery) to only treat null/undefined as empty by supplying a custom
emptiness checker so an empty array still renders the journal UI. Locate
DataState with journalQuery and add an isEmpty (or equivalent prop supported by
DataState) that returns true only when data == null/undefined (e.g., isEmpty:
data => data == null) so WorkoutJournalSection sees an empty workouts array and
can render its own empty UI.

@github-actions github-actions Bot added the size/M label May 4, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 4, 2026

⏱️ CI Pipeline Duration Report

Based on the last 50 successful runs on the default branch.

Overall Pipeline

Metric Value
p50 6m 26s
p95 7m 55s
p99 9m 3s
Current run 10m 4s
vs p95 +27.2%

Trend (last 20 runs): ▃▃▁▂▃▃▃▂▃▃▂▂▄▃▃▆▅▄█▆

Per-Job Breakdown

Job p50 p95 p99 Current vs p95
Accessibility (axe-core) 2m 5s 2m 21s 2m 23s 0s -100.0%
Commit messages (commitlint) 0s 0s 0s 35s N/A
Critical-flow E2E (Playwright) 1m 36s 1m 44s 1m 44s 6m 27s +272.1%
Migration lint (AGENTS rule 0s 0s 0s 8s N/A
Pipeline duration (p95 trend) 26s 27s 27s
Secret scan (gitleaks) 8s 11s 11s 12s +9.1%
Smoke E2E (Playwright) 1m 26s 1m 40s 1m 40s
Test coverage (vitest) 2m 4s 2m 33s 2m 33s 2m 12s -13.7%
Workflow lint (actionlint) 7s 7s 7s 8s +14.3%
check 4m 12s 4m 54s 5m 6s 57s -80.6%
tsconfig strict guard (PR-1.A) 5s 14s 14s 6s -57.1%

⚠️ Warning: Current run (10m 4s) exceeds p95 + 20% threshold (9m 30s). Consider reviewing slow jobs.

@Skords-01 Skords-01 merged commit 29de82b into main May 4, 2026
27 of 50 checks passed
@Skords-01 Skords-01 deleted the devin/1777905144-fizruk-datastate-adoption branch May 4, 2026 15:02
Skords-01 pushed a commit that referenced this pull request May 4, 2026
Update initiative 0011 to reflect Phase 2 progress through 2026-05-04:

- Status header bumped to include the four open <DataState> consumer
  PRs: 2.4 #1703 (finyk), 2.5 #1709 (fizruk), 2.6 #1713 (nutrition),
  2.7 #1714 (routine). Last-validated handle aligned with the actual
  initiative owner (@Skords-01).
- Phase 2 table column 'Файли (приклад)' renamed to 'Файли (фактичні
  споживачі)' and ETA column replaced with Status (PR-link). Rows for
  2.4–2.7 now point at the actual landed targets, not the initial
  guess names from the proposal draft (MonoTransactionsPanel,
  WorkoutHistoryPanel, NutritionMealsPanel, RoutineList, etc. — none
  of those components physically exist in the repo).
- New explanatory note after the table records why the file list
  diverged from the original draft (real Skeleton-based loading sites
  per module) and documents the per-module finding for 2.6 / 2.7
  (NutritionApp.tsx Menu plan branch; RoutineTimeline.tsx calendar
  branch — both modules have exactly one consumer of
  @shared/components/ui/Skeleton).

Supersedes #1710 (single doc PR for 2.4 + 2.5 only); this combined
update is the cleaner record because all four <DataState> consumer
adoption PRs landed on the same day.

Closes the doc-side of initiative 0011 PR 2.6 + 2.7.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant