refactor(chat): extract composer core to @orbit/shared (#85) - #130
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
One hard-rule violation in the otherwise excellent extraction work.
composerReducer / initialComposerState / ComposerState / ComposerAction (lines 167–194, packages/shared/src/hooks/chat-composer-core.ts) are dead exports — no app code imports them. Both hooks still wire sendError and showLangPicker via useState, so the reducer is untouched outside its own test file. This is a direct CLAUDE.md rule 2 violation ("No 'just in case' exports"). Fix: either delete the block and its test cases, or actually migrate the app hooks to useReducer(composerReducer, initialComposerState) so the export is load-bearing. Inline comment with both options posted on the relevant lines.
There was a problem hiding this comment.
Two dead exports remain in the new shared core after the reducer cleanup from the previous round.
CHAT_HABIT_ACTION_TYPES (line 33) and AGENT_INVALIDATION_KEY_FAMILIES (line 78) in packages/shared/src/hooks/chat-composer-core.ts are both exported but have zero callers outside the file that defines them. Both violate CLAUDE.md rule 2 (no 'just in case' exports). Fix is a one-character drop of the export keyword on each — callers go through selectActionInvalidations and invalidateAgentQueries respectively, which remain exported.
Everything else in this PR is solid: the reducer dead-code from the prior round is fully removed, the shared core is pure TS with no React/RN imports, both platforms consume the shared functions, tests cover the core logic and the mobile hook, and the chat screen slimming is clean.
There was a problem hiding this comment.
Both flagged rounds are fully resolved. The latest commit correctly drops the remaining internal-only export keywords from chat-composer-core.ts (CHAT_HABIT_ACTION_TYPES, AGENT_INVALIDATION_KEY_FAMILIES, SendFailureKind, SendFailureInput, SendFailureClassification, ActionInvalidations), so the file now exports exactly what external callers use: buildAgentExecutionMessage, classifySendFailure, findPremiumPolicyDenial, invalidateAgentQueries, selectActionInvalidations, CHAT_GOAL_ACTION_TYPES, and CHAT_DRAFT_STORAGE_KEY. The shared core is clean pure-TS with no React/RN imports, both thin wrappers consume it correctly, the chat screen slim-down is clean, and the test suite (shared core unit + mobile hook integration) gives solid coverage of the extracted logic. Ready to merge.
Lift the chat-composer logic into a framework-agnostic core in packages/shared so any chat change lands once and both apps inherit it. - Add packages/shared/src/hooks/chat-composer-core.ts: pure TS (no react / react-native at runtime) owning the de-duped action-type sets, buildAgentExecutionMessage (web-rich, keeps policyReason), classifySendFailure (408/limit/upgrade/generic), selectActionInvalidations, findPremiumPolicyDenial, the agent-query invalidation set, and the composer reducer. Exported via the index + new "./hooks" package subpath. - Rewrite apps/web/hooks/use-chat-composer.ts as a thin wrapper injecting the chat Server Actions; web chat page now consumes pre-shaped step-up adapters. - Add apps/mobile/hooks/use-chat-composer.ts (thin apiClient wrapper) + use-chat-reward.ts (mobile-only rewarded-ad flow). - Slim apps/mobile/app/chat.tsx from 1544 to 286 lines: extract createStyles, the 3 animation components, ChatComposerInput, ChatInputBar, ChatInputArea, and ChatEmptyState to siblings. - Normalize divergences to web's richer behavior: mobile gains policyReason and the timeout/limit error branches; both platforms use the shared CHAT_GOAL_ACTION_TYPES. Tests: shared core unit test + mobile hook test added; all suites green. Closes #85 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Three rounds of review all resolved. The shared core is clean: CHAT_HABIT_ACTION_TYPES, AGENT_INVALIDATION_KEY_FAMILIES, and the internal discriminated-union types (SendFailureKind, SendFailureInput, SendFailureClassification, ActionInvalidations) are all module-private — no export leakage. The five public exports (buildAgentExecutionMessage, classifySendFailure, findPremiumPolicyDenial, invalidateAgentQueries, selectActionInvalidations) plus CHAT_GOAL_ACTION_TYPES and CHAT_DRAFT_STORAGE_KEY are all load-bearing in both platform hooks. The shared core is pure TypeScript with zero React/RN imports, both thin wrappers consume it correctly with the divergence normalization (policyReason, 408/limit error branches) landing on both platforms, the chat screen slim-down from 1544 → 286 lines is clean, and the test suite gives solid coverage of the extracted logic.
|
1 participant
Closes #85
What
Lifts the chat-composer logic into a framework-agnostic core in
packages/sharedso any chat change is written once and both web and mobile inherit it (previously a 537-line web hook and 1544-line mobile screen drifted independently —policyReason, error classification, and upgrade resolution all differed).Deviation from the issue's literal acceptance criteria (F4 = A, pure-core)
The issue asks for the core at
packages/shared/hooks/use-chat-composer-core.tsas a "platform-agnostic state machine." Butpackages/shared/CLAUDE.mdmandates pure TypeScript — no React/React Native imports. Ause*hook callinguseState/useCallbackwould import React into the shared package and violate that rule.Reconciliation:
packages/shared/src/hooks/chat-composer-core.ts(every shared module lives undersrc/and is barreled throughsrc/index.ts; reconciles the issue'spackages/shared/hooks/path).use*hook: the de-duped action-type sets,buildAgentExecutionMessage,classifySendFailure,selectActionInvalidations,findPremiumPolicyDenial, the agent-query invalidation set, and the composer reducer. Zeroreact/react-nativeimports (QueryClientis typed from@tanstack/query-core, already a shared dep).useChatComposerhooks wire React state + inject I/O (web: chat Server Actions; mobile:apiClient).Divergence normalization (mobile adopts web's richer behavior)
buildAgentExecutionMessagekeepsoperation.policyReasonin the fallback chain on both platforms (mobile previously omitted it).apiClienterror into{ status, code, reason }viaextractBackendStatus/extractBackendError/extractBackendErrorCodebefore calling the sharedclassifySendFailure(mobile previously only had the upgrade branch).CHAT_GOAL_ACTION_TYPESfor the goal/habit drawer routing.Platform-only adapters stay per-app: web textarea autosize / DOM paste /
localStoragedraft; mobile AdMob rewarded ad (use-chat-reward.ts),useOfflinegating,AsyncStoragedraft, Android keyboard insets.Mobile screen slimming
apps/mobile/app/chat.tsx: 1544 → 286 lines. ExtractedcreateStyles→chat.styles.ts; the 3 animation components →chat-animations.tsx;ChatComposerInput,ChatInputBar,ChatInputArea,ChatEmptyState→components/chat/*; rewarded-ad flow →hooks/use-chat-reward.ts.Tests & validation
classifySendFailure408/403/upgrade/generic,buildAgentExecutionMessageprecedence,selectActionInvalidations) and mobile hook test (7 cases — send success appends + usage bump, 403 premium → upgrade route, 403 non-upgrade → limit error, offline guard, pending-op confirm→execute endpoints).npm run type-check✅ (3/3),npm run lint✅ (2/2),npm test✅ (shared 830, web 1448, mobile 368 — green per workspace; turbo serial run 3/3).Manual E2E pending
No browser E2E here. Manual E2E pending for the chat smoke flows on both platforms: image upload (pick/paste/validate → attach), history paging, tool-call / pending-operation rendering, retry, and error states (timeout / limit / upgrade-redirect / generic).
🤖 Generated with Claude Code