refactor: split god files per AGENTS.md conventions (#67)#68
Merged
pablopunk merged 2 commits intoJun 12, 2026
Merged
Conversation
Split three large files into focused modules following the queue.model.ts barrel-reexport pattern: - src/actions/projects.ts (966→33 lines): 10 focused action modules (create, crud, delete, lifecycle, production, settings, prompt, queue-status, production-history, opencode) - src/hooks/useChatPanel.ts (1136→281 lines): 6 focused hooks (scroll, sse, restore, send, actions, history) - src/stores/useChatStore.ts (648→379 lines): 4 handler modules (messageHandlers, sessionHandlers, interactionHandlers, toolHandlers) Pure refactor — zero behavior changes. All barrel re-exports preserve backward compatibility for astro:actions callers and hook imports.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 36 minutes and 11 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
WalkthroughThis PR performs a pure refactoring of three large modules into focused modules per AGENTS.md conventions. Sequence Diagram(s)No sequence diagram is generated for this change set because it is a pure refactoring with zero behavioral modifications. The control flows, component interactions, and message sequences remain unchanged; only the physical file organization has shifted. 🚥 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. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Nitpick comments (1)
src/stores/useChatStore.interactionHandlers.ts (1)
12-18: ⚡ Quick winKeep these handlers typed at the boundary instead of re-casting
Record<string, unknown>.The extracted module currently throws away the event payload shape and then reintroduces it with
as unknown as .... That defeats strict-mode checking and makes payload drift invisible during future refactors. Prefer concrete payload interfaces on the handler signatures instead.As per coding guidelines, "Avoid
anytypes when possible; everything should be typed correctly without type duplication or casting" and "Use TypeScript throughout with strict mode enabled."Also applies to: 24-30, 36-41
🤖 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 `@src/stores/useChatStore.interactionHandlers.ts` around lines 12 - 18, The handler functions (e.g., handlePermissionRequested) are discarding payload types by accepting Record<string, unknown> and re-casting to specific interfaces; change each handler signature to accept the concrete payload type (e.g., PendingPermissionRequest) instead of Record<string, unknown>, remove the `as unknown as ...` cast and set the state directly (e.g., set({ pendingPermission: request })), and apply the same fix to the other handlers referenced (the handlers at the next two blocks) so payload shapes are preserved at the boundary and TypeScript strict checks remain effective.Source: Coding guidelines
🤖 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 `@src/actions/projects.create.ts`:
- Around line 32-45: The code silently swallows JSON parse errors for
input.attachments causing lost user files; update the parsing logic in the
projects.create action to JSON.parse inside a try/catch and on any parse or
schema/typing mismatch throw an ActionError with a BAD_REQUEST code; validate
that the parsed value is an array and each item has filename:string,
mime:string, dataUrl:string and optional kind ("image"|"text") and
textContent:string when present, and assign to the attachments variable only
after successful validation (use the existing attachments identifier and
input.attachments, and throw new ActionError('BAD_REQUEST', 'Invalid attachments
JSON') or similar).
In `@src/actions/projects.delete.ts`:
- Around line 33-42: The pre-delete coordination currently swallows failures in
updateProjectStatus and cancelActiveProductionJobs; change this so failures
abort the delete flow instead of being ignored: remove the empty catch blocks
and ensure updateProjectStatus(input.projectId, "deleting") errors are
propagated (or throw a new error with context) so the action returns failure,
and similarly import and call cancelActiveProductionJobs(input.projectId) must
surface errors (log context and rethrow) rather than swallowing them; ensure
both updateProjectStatus and cancelActiveProductionJobs are awaited and any
thrown error stops the deletion process so callers can react.
In `@src/actions/projects.lifecycle.ts`:
- Around line 74-79: The code currently calls
updateProjectStatus(input.projectId, "starting") before
enqueueDockerEnsureRunning(...), which can leave the project in a persistent
"starting" state if enqueue fails; change the flow so that you first call
enqueueDockerEnsureRunning({ projectId: input.projectId, reason: "user" }) and
only if that promise resolves successfully call
updateProjectStatus(input.projectId, "starting"); if enqueue fails, do not
change the persisted status (or explicitly revert it to its previous value) and
surface/log the enqueue error so callers can retry; update references to
updateProjectStatus and enqueueDockerEnsureRunning in this block accordingly.
In `@src/actions/projects.opencode.ts`:
- Around line 21-35: The current ownership check using isProjectOwnedByUser is
insufficient because restartGlobalOpencode() affects global shared state;
replace the project-owner guard with an elevated-permission check (e.g., call
isSystemAdmin(user.id) or isOrgAdmin(user.id, input.orgId)) and only allow the
restart when that admin check passes, otherwise throw a forbidden ActionError
(code "FORBIDDEN" or similar). Locate the call site around isProjectOwnedByUser
and restartGlobalOpencode and update the logic to verify the elevated role (or
use a dedicated isAuthorizedForGlobalRestart function), returning/throwing
appropriately instead of allowing any project owner to call
restartGlobalOpencode. Ensure the log and returned error reflect the permission
failure.
In `@src/actions/projects.production.ts`:
- Around line 48-64: The code sets the production status to "queued" via
updateProductionStatus(projectId, "queued", ...) before calling
enqueueProductionBuild, which can throw and leave the project stranded; modify
the flow in the function that calls hasActiveDeployment, updateProductionStatus,
and enqueueProductionBuild so that if enqueueProductionBuild({ projectId })
throws you catch the error, call updateProductionStatus(projectId, "idle" or
previous status, { productionError: <error> or null }) to roll back the queued
state, then rethrow or wrap the error (preserving ActionError behavior); ensure
the rollback runs only on enqueue failure and reference the existing functions
hasActiveDeployment, updateProductionStatus, and enqueueProductionBuild when
implementing the try/catch cleanup.
- Around line 231-247: The code currently swallows symlink update errors (around
tempSymlink/hashPath/symlinkPath) but always calls updateProductionStatus with
the new productionHash and running state; move the updateProductionStatus(...)
call so it only runs after the filesystem operations succeed (i.e., inside the
try after fs.rename), or alternatively set a success flag when rename completes
and check it before calling updateProductionStatus; ensure you reference the
same symbols (tempSymlink, hashPath, symlinkPath, getCanonicalProductionUrl,
updateProductionStatus, input.projectId, input.toHash, project) and do not
persist the running/productionHash state if the symlink swap failed (either
rethrow or log and abort the DB update).
In `@src/actions/projects.queue-status.ts`:
- Around line 138-140: The code sets promptSentAt for any job with jobType ===
"opencode.sendUserPrompt" regardless of outcome; update the logic in the block
that handles jobType to only assign promptSentAt when the job indicates a
successful/finished send (e.g., check the job's success/completed status such as
job.status === "completed" or the presence of a finished/finishedAt timestamp)
so failed/queued/running jobs do not populate promptSentAt; locate the
assignment to promptSentAt and gate it behind that success check for the
opencode.sendUserPrompt case.
In `@src/actions/projects.settings.ts`:
- Around line 33-43: updateProjectModel(...) already writes opencode.json, so
remove the redundant updateOpencodeJsonModel(...) call and its try/catch;
instead call updateProjectModel(input.projectId, input.model) inside a single
try/catch (in the containing updateModel handler) and on error log a warning via
logger.warn mentioning the failure from updateProjectModel so the fallback
catches the actual failure path; keep references to updateProjectModel and
updateOpencodeJsonModel so you remove only the extra call and ensure errors from
updateProjectModel are handled by the surrounding try/catch.
In `@src/hooks/useChatActions.ts`:
- Around line 137-170: The code clears image attachments (pendingAttachments via
setPendingAttachments) before calling actions.projects.updateModel, but only
rolls back currentModel on failure; either defer clearing until the update
succeeds or save and restore the previous attachments on failure: capture the
current pendingAttachments in a local variable (e.g., previousAttachments)
before modifying, then when actions.projects.updateModel returns success only
then call setPendingAttachments(textAttachments) and
setPendingAttachmentError(null); alternatively, if you prefer to keep
early-clear, restore previousAttachments in both the !result.data?.success
branch and the catch block alongside setCurrentModel(previousModel) so queued
images are not lost. Ensure you reference newModelSupportsAttachments,
pendingAttachments, setPendingAttachments, setPendingAttachmentError,
actions.projects.updateModel and setCurrentModel when making the change.
In `@src/hooks/useChatHistory.ts`:
- Around line 143-146: The session metadata-loaded flags are initialized
incorrectly: sessionContextLoaded is always false which causes client fetches to
run even when SSR-hydrated data exists; update the initialization in
useChatHistory so sessionTitleLoaded and sessionContextLoaded are seeded from
the hydrated store/state (e.g., derive initial values from sessionTitle and any
existing sessionContext/sessionMessages) instead of hardcoding false, and add a
guard in the effect that fetches metadata/messages to bail out when those seeded
flags indicate metadata is already present (use
sessionContextLoaded/sessionTitleLoaded and
setSessionContextLoaded/setSessionTitleLoaded to reflect the hydrated state);
apply the same change for the similar block around lines referenced (291-320) to
prevent unnecessary refetches.
In `@src/hooks/useChatSend.ts`:
- Around line 61-82: The local messageParts array is built as attachments then
text while the API payload is built as text then attachments, causing
inconsistent ordering; fix by deriving both the local MessagePart[]
(messageParts) and the API payload parts from a single ordered source: build a
unified parts array by iterating a single sequence that respects the original
user order (e.g., an ordered array of items where each item is either an
attachment or text) and map each item to createPromptAttachmentPart or
createTextPart as appropriate (update the logic in useChatSend.ts where
messageParts, createPromptAttachmentPart, and createTextPart are created; apply
the same unified approach to the other occurrence around the code referenced at
the second occurrence). Ensure the same array is used for both local rendering
and the API payload so ordering remains identical.
In `@src/hooks/useChatSse.ts`:
- Around line 101-116: The reconnect logic currently still schedules a
setTimeout to call connect() even after reconnectAttemptsRef.current >
MAX_SSE_RECONNECT_ATTEMPTS; change the flow in the block that checks
MAX_SSE_RECONNECT_ATTEMPTS (where reconnectAttemptsRef, setIsStreaming,
streamDegradedRef, and toast are used) to return early after marking the stream
degraded and setting isStreaming false so no reconnect is scheduled; also clear
any existing reconnectTimeoutRef (clearTimeout) before returning to ensure no
pending timers remain; this prevents further calls to connect() (the connect
function) when opencodeReady and avoids hammering the SSE endpoint.
- Around line 45-56: The inactivity watchdog in scheduleInactivityTimeout is
being armed regardless of whether a response generation is actually in flight;
update scheduleInactivityTimeout and its call sites (e.g., where chat.event
handlers and connection open logic call it) to only start the timeout when a
streaming flag is set (use isStreaming state or introduce streamingRef.current)
and never arm it when no generation is active; ensure clearInactivityTimer is
called and streamDegradedRef handling remains when a stream ends
(setIsStreaming(false) -> clearInactivityTimer), and apply the same gating logic
to the other occurrences mentioned (lines ~64-80) so the watchdog only runs
during real streaming.
In `@src/stores/useChatStore.messageHandlers.ts`:
- Around line 29-35: The handler currently ignores the incoming partId and
always mutates the first text part; update the existing-message branch in the
message update handler to locate the specific part by matching part.id ===
partId on message.parts (checking part.type === 'text') and then apply
deltaText/text to that part instead of the first match; likewise apply the same
partId-based lookup to the other similar branch (lines 44-56) so updates target
the correct message part per the partId contract used in normalize.ts and
types/message.ts.
- Around line 222-227: The constructed nextMessage currently always sets
localStatus: "sent" even when an error exists; update the logic in the
nextMessage creation (the object built using Message, parts/nextParts,
isStreaming, localStatus, localError) to set localStatus to "failed" when error
is present (e.g., localStatus: error ? "failed" : "sent") and keep the existing
localError assignment so errored completions are clearly marked per the Message
contract.
In `@src/stores/useChatStore.toolHandlers.ts`:
- Around line 17-24: The update handler currently destructures toolCallId, name,
input, status, output, error from the payload and overwrites the stored tool
entry with these values even when input/output/error are undefined; change the
update logic in useChatStore.toolHandlers.ts (the handler that processes the
normalized tool update payload) to first locate the existing tool by toolCallId
and merge fields so that input, output, and error are only replaced when they
are present in the payload (i.e., use the existingTool.input/output/error when
the payload value is undefined), and apply the same merging behavior in the
other branch referenced around lines 39-45 to preserve prior tool fields per the
normalized payload contract.
---
Nitpick comments:
In `@src/stores/useChatStore.interactionHandlers.ts`:
- Around line 12-18: The handler functions (e.g., handlePermissionRequested) are
discarding payload types by accepting Record<string, unknown> and re-casting to
specific interfaces; change each handler signature to accept the concrete
payload type (e.g., PendingPermissionRequest) instead of Record<string,
unknown>, remove the `as unknown as ...` cast and set the state directly (e.g.,
set({ pendingPermission: request })), and apply the same fix to the other
handlers referenced (the handlers at the next two blocks) so payload shapes are
preserved at the boundary and TypeScript strict checks remain effective.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f42aa2ad-39f8-4916-a0c2-93cce4819c43
📒 Files selected for processing (23)
src/actions/projects.create.tssrc/actions/projects.crud.tssrc/actions/projects.delete.tssrc/actions/projects.lifecycle.tssrc/actions/projects.opencode.tssrc/actions/projects.production-history.tssrc/actions/projects.production.tssrc/actions/projects.prompt.tssrc/actions/projects.queue-status.tssrc/actions/projects.settings.tssrc/actions/projects.tssrc/hooks/useChatActions.tssrc/hooks/useChatHistory.tssrc/hooks/useChatPanel.tssrc/hooks/useChatRestore.tssrc/hooks/useChatScroll.tssrc/hooks/useChatSend.tssrc/hooks/useChatSse.tssrc/stores/useChatStore.interactionHandlers.tssrc/stores/useChatStore.messageHandlers.tssrc/stores/useChatStore.sessionHandlers.tssrc/stores/useChatStore.toolHandlers.tssrc/stores/useChatStore.ts
- Validate attachments JSON in project create (throw BAD_REQUEST) - Enqueue before status change in restart/deploy (avoid stranded state) - Restore attachments on model-change failure in useChatActions - Seed sessionContextLoaded from hydrated sessionTitle - Consistent ordering between local message parts and API payload - Halt SSE reconnect after max attempts exceeded - Respect partId when updating text parts in message handlers - Set localStatus: 'failed' when message final carries an error - Merge tool update fields instead of overwriting with undefined - Type interaction handlers with concrete payload types
Owner
Author
CodeRabbit review resolutionFixed in 05555c6:
Intentionally skipped (design decisions / pre-existing patterns):
|
pablopunk
deleted the
fractal-doce-dev-work-67-split-god-files-per-agents-md-385b48
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
1 participant
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.
Closes #67
Splits three large files into focused modules following the
queue.model.tsbarrel-reexport pattern from the codebase.Changes
src/actions/projects.tssrc/hooks/useChatPanel.tssrc/stores/useChatStore.tsNew files (20)
projects.create.ts,projects.crud.ts,projects.delete.ts,projects.lifecycle.ts,projects.production.ts,projects.settings.ts,projects.prompt.ts,projects.queue-status.ts,projects.production-history.ts,projects.opencode.tsuseChatScroll.ts,useChatSse.ts,useChatRestore.ts,useChatSend.ts,useChatActions.ts,useChatHistory.tsuseChatStore.messageHandlers.ts,useChatStore.sessionHandlers.ts,useChatStore.interactionHandlers.ts,useChatStore.toolHandlers.tsGround rules followed
astro:actions,ChatPanel.tsx, etc.)pnpm format+pnpm buildpassSummary by CodeRabbit
New Features
Refactor