refactor(web): adopt <DataState> in HubChat / coach / digest#1726
Merged
Skords-01 merged 1 commit intoMay 4, 2026
Merged
Conversation
Перевіряння всіх Skeleton-споживачів у `apps/web/src/core/**` показало, що єдине місце з канонічним 4-станним loading-ladder-ом (skeleton → error → empty → content) — `WeeklyDigestCard.DigestContent`. Усі інші "HubChat / coach / digest"-related точки з пропозиції PR 2.8: - `AssistantAdviceCard` — без skeleton, лише inline `Готую пораду…` text. Завжди має локальний кеш (last-good insight). DataState не потрібен. - `HubChatHistoryDrawer` — local-first, sessions передаються пропом. Empty-state inline. - `HubChat.tsx` / `HubChatBody` / `HubChatComposer` — no Skeleton import, стрімінг-стейт повідомлень. Тому скоуп цього PR — `DigestContent` 4-state ladder → `<DataState>`. Поведінка 1:1 з оригіналом: - `loading=true` форсує `data: undefined`, що тригерить skeleton-слот (= попередній `if (loading) return <LoadingSpinner />` short-circuit; стале digest приховується під час regen). - `error && !loading` форсує `isError: true`, DataState's "error wins" гілка вибирає error-слот (= попередній `if (error)`). - `data: digest ?? null` + `isEmpty(d) = !d || !(d.finyk||...)` дає empty-слот для no-digest або digest-without-modules. - Інакше — children(d) рендерить existing content body. Додано `WeeklyDigestCard.datastate.test.tsx` (5 кейсів × 4 стани + empty-past-week варіант) із RTL-cleanup після кожного render-у (apps/web vitest setup не реєструє auto-cleanup за замовчуванням). Refs initiative `docs/initiatives/0011-foundation-adoption-and-process-discipline.md` PR 2.8.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR refactors ChangesWeeklyDigestCard DataState Adoption
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 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)
Review rate limit: 5/10 reviews remaining, refill in 26 minutes and 9 seconds. Comment |
9 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@apps/web/src/core/insights/WeeklyDigestCard.tsx`:
- Around line 248-253: digestQuery's error field is set unconditionally which
can make DataState render the error slot during loading if a prior error exists;
update the construction of digestQuery (type DataStateQueryLike) so the error is
only populated when not loading (e.g. set error to null while loading) — keep
isLoading as loading and isError as !!error && !loading, but change error to be
conditional on !loading (e.g. error: !loading && error ? new Error(error) :
null) so the skeleton has priority during regeneration.
🪄 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: e2d2b37c-057a-4f39-887b-ad9474b8ff39
📒 Files selected for processing (2)
apps/web/src/core/insights/WeeklyDigestCard.datastate.test.tsxapps/web/src/core/insights/WeeklyDigestCard.tsx
Comment on lines
+248
to
+253
| const digestQuery: DataStateQueryLike<DigestPayload | null> = { | ||
| data: loading ? undefined : (digest ?? null), | ||
| isLoading: loading, | ||
| isError: !!error && !loading, | ||
| error: error ? new Error(error) : null, | ||
| }; |
There was a problem hiding this comment.
Potential bug: error slot may render during loading if a previous error exists.
The comment explains that the skeleton should have priority during regeneration, but the error prop is set unconditionally when error is truthy. DataState's "error wins" check evaluates isError === true || queryError != null, so a non-null error will trigger the error slot even when isError is false.
If React Query's useMutation clears mutation.error when a new mutation starts, this works by coincidence. However, the component shouldn't rely on that implicit behavior.
Proposed fix: guard error prop with loading check
const digestQuery: DataStateQueryLike<DigestPayload | null> = {
data: loading ? undefined : (digest ?? null),
isLoading: loading,
isError: !!error && !loading,
- error: error ? new Error(error) : null,
+ error: !loading && error ? new Error(error) : null,
};📝 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
| const digestQuery: DataStateQueryLike<DigestPayload | null> = { | |
| data: loading ? undefined : (digest ?? null), | |
| isLoading: loading, | |
| isError: !!error && !loading, | |
| error: error ? new Error(error) : null, | |
| }; | |
| const digestQuery: DataStateQueryLike<DigestPayload | null> = { | |
| data: loading ? undefined : (digest ?? null), | |
| isLoading: loading, | |
| isError: !!error && !loading, | |
| error: !loading && error ? new Error(error) : null, | |
| }; |
🤖 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 `@apps/web/src/core/insights/WeeklyDigestCard.tsx` around lines 248 - 253,
digestQuery's error field is set unconditionally which can make DataState render
the error slot during loading if a prior error exists; update the construction
of digestQuery (type DataStateQueryLike) so the error is only populated when not
loading (e.g. set error to null while loading) — keep isLoading as loading and
isError as !!error && !loading, but change error to be conditional on !loading
(e.g. error: !loading && error ? new Error(error) : null) so the skeleton has
priority during regeneration.
⏱️ CI Pipeline Duration ReportBased on the last 50 successful runs on the default branch. Overall Pipeline
Trend (last 20 runs): Per-Job Breakdown
|
Skords-01
pushed a commit
that referenced
this pull request
May 4, 2026
Status header: додано PR 2.8 (#1726) і змінено сигнал з "2.8 HubChat/coach/digest залишається" на "2.9 ESLint rule і 2.1 ManualExpenseSheet залишаються" — consumer-adoption блок Phase 2 закрито. Phase 2 table row 2.8: ETA "+3 дні" → "Opened 2026-05-04 — #1726", файл `core/insights/WeeklyDigestCard.tsx` `DigestContent` 4-state ladder як єдиний Skeleton-based panel-loading site у HubChat / coach / digest зоні `core/**`. Footnote: додано пояснення per PR 2.8 чому інші HubChat / coach / digest "панелі" з пропозиції (`HubChatHistoryPanel`, `CoachInsightsPanel`, `DigestPanel`) не мали реальних DataState- targets — `AssistantAdviceCard` без skeleton imports і кешує last-good insight, `HubChatHistoryDrawer` local-first, `HubChat.tsx` / `HubChatBody` / `HubChatComposer` стрімлять без panel-skeleton-у. Refs initiative 0011 PR 2.8.
Skords-01
deleted the
devin/1777916279-hubchat-coach-digest-datastate-adoption
branch
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.
Summary
Адаптуємо
<DataState>у HubChat / coach / digest зоніapps/web/src/core/**. Інспекція всіх Skeleton-споживачів і панельних loading-ladder-ів показала, що єдина точка з канонічним 4-станним контрактом (skeleton → error → empty → content) —WeeklyDigestCard.DigestContent. Інші компоненти з пропозиції 2.8 не мають panel-level skeleton-у:AssistantAdviceCard(core/insights/AssistantAdviceCard.tsx) — inline<p>Готую пораду…</p>безSkeletonimport. Завжди має кеш last-good insight (черезsafeReadStringLS), тож loading-стейт є transient inline-аффордансом, не панельним skeleton-ом.HubChatHistoryDrawer(core/hub/HubChatHistoryDrawer.tsx) — local-first;sessionsприходить пропом, без async fetch / Skeleton import. Empty-state — inline mini-state у scrollable списку.HubChat.tsx/HubChatBody.tsx/HubChatComposer.tsx— стрімінг-стейт повідомлень, безSkeletonimport;isPendingкерує SSE-progress UI, не панельним skeleton-swap-ом.Тому скоуп цього PR —
WeeklyDigestCard.DigestContent4-state ladder →<DataState>(єдинийSkeleton-споживач уcore/insights). Це закриває весь<DataState>consumer-adoption блок Phase 2 ініціативи0011.Що змінилось
apps/web/src/core/insights/WeeklyDigestCard.tsx:DataState+DataStateQueryLike.DigestContent(внутрішній компонент) переписаний з 3 ранніхreturn-ів (if (loading) ... if (error) ... if (!hasData) ...) на єдиний<DataState>-wrapper з 4 слотами.LoadingSpinner(Skeleton/SkeletonTextmatched-shape loader на 4 module-row-и) залишений без змін, передається якskeleton.error/emptyслоти — JSX-константи у closure, мають доступ доisCurrentWeek/onGenerateчерез scope.isEmptychecker:(d) => !d || !(d.finyk || d.fizruk || d.nutrition || d.routine)— повторює превhasDataлогіку (digest існує і має data хоча б у одному модулі).children: (d) => <>...</>— використовуєdзамість propdigest.Truth-table (поведінка 1:1 з prev)
loadingerrordigest(hasData)isCurrentWeek<LoadingSpinner />Mapping:
Force
data: undefinedпід час loading зберігає попереднійif (loading)short-circuit (стале digest приховується під час regen).isError: !!error && !loadingгарантує, що під час regen-error spinner показується першим (як раніше), а error-block тільки після того, як loading осідає.Тести
apps/web/src/core/insights/WeeklyDigestCard.datastate.test.tsx(новий, 5 кейсів):loading=true→aria-busyskeleton; жодного "Згенерувати" / "Спробувати знову" афорданс-у.loading=false, digest=null, isCurrentWeek=true→ empty-slot з кнопкою "Згенерувати звіт".isCurrentWeek=false→ empty-slot з текстом "Звіт за цей тиждень не збережено", БЕЗ "Згенерувати" кнопки.loading=false, error="..."→ error-slot з retry-кнопкою.loading=false, digest={finyk: {...}}→ content body з "Переглянути як сторіс" CTA.Тести явно реєструють
afterEach(cleanup)— apps/web vitest setup (src/test/setup.ts) не реєструє RTL auto-cleanup, тож без cleanup-у попередній render leaks DOM в наступний кейс.Існуючий
WeeklyDigestCard.collapse.test.tsx(2 кейси на header collapse-control) проходить без змін.Governing Skill
.agents/skills/sergeant-web-ui/SKILL.md.agents/skills/sergeant-feature-delivery/SKILL.md(для DataState consumer-adoption pattern, який розкатуємо інкрементально по модулях)Playbook
<DataState>контракт. Жодних playbook-ів для DS migration sweep-ів немає (поки що); патерн фіксується PRs 2.4–2.7 як reference.0011Phase 2 work, який зараз триває; playbook буде створено після того, як ESLint rule (PR 2.9) буде enforced.Verification
Additional checks:
Docs and Governance
AGENTS.mdneeded an update. (no change — DataState contract уже задокументовано в попередніх PR)sergeant-web-uiskill вже описує DataState; новий — в окремому doc PR після того, як 2.9 ESLint rule landings)Updated docs:
docs(docs): record 0011 PR 2.8 ...PRRisk and Rollout
<div>навколо рендереного слоту (без класу), що може вплинути на flexbox-edge-cases у parent-і. Перевірено: parent (WeeklyDigestCardouter card) — block-levelrounded-2xl border bg-panel, всі діти — звичайні block-level<div>-и. Extra wrapper не змінює layout.WeeklyDigestCard.tsxповерне 3 ранніхreturn-и. Тест-файл можна видалити окремо або залишити (він valid для будь-якої реалізації, поки контракт-prop-и DigestContent stable).Hard Rule #15
AGENTS.mdbefore coding.--no-verify.Reviewer Notes
errorслот — function-form() => errorSlot(а неerrorSlotяк ReactNode), бо TS не звужує union без callsite, але runtime однаково (DataState не передаєerr/retryуerrorSlot).isError: !!error && !loading— навмисно False під час loading, щоб preserve "spinner wins" precedence з оригіналу. Якщо рев'юер вважає, що error-during-regen має показуватись одразу — змінити на!!errorі прибрати&& !loading.WeeklyDigestCard.datastate.test.tsxявно додаєafterEach(cleanup)— це local fix; глобальний RTL auto-cleanup для всіх web-тестів — окремий refactor (поза скоупом цього PR).apiV1.test.ts, mobile buildvite buildvs@sergeant/db-schema/sqlite, E2E auth, Argos visual regression) — не від цього PR; той самий набір падає на 2.4 (refactor(web): adopt <DataState> in finyk Mono panels #1703), 2.5 (refactor(web): adopt <DataState> in fizruk Workouts journal #1709), 2.6 (refactor(web): adopt <DataState> in nutrition panels #1713), 2.7 (refactor(web): adopt <DataState> in routine panels #1714).Summary by cubic
Refactored
WeeklyDigestCard.DigestContentto use@shared/components/ui/DataState, unifying the 4-state flow (skeleton → error → empty → content) without changing behavior. Adds focused tests to cover all states.<DataState>and aDataStateQueryLikemapping:data=undefinedwhile loading,isErroronly when not loading,errorpassed through.LoadingSpinnerasskeleton; moved body tochildren(d); addedisEmptyto mirror previoushasDatacheck.apps/web/src/core/insights/WeeklyDigestCard.datastate.test.tsxwith 5 cases (skeleton, empty current/past, error, content) and explicit RTL cleanup.Written for commit d2152bb. Summary will update on new commits.
Summary by CodeRabbit
Tests
Refactor