revert(desktop): revert unrelated changes in mastra chat file support#2111
Merged
Conversation
This reverts commit b528ad3.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a git-task-worker entry and routes Git operations through runGitTask; implements debounced tRPC storage writes; extends resource metrics with host data and caching; adds multi-provider AI naming; and applies numerous UI, chat, storage, and store refactors across the desktop app. Changes
Sequence Diagram(s)sequenceDiagram
participant Renderer as Renderer (UI)
participant TRPC as TRPC Router
participant Main as Electron Main
participant Worker as git-task-worker
participant Git as Git CLI/FS
Renderer->>TRPC: request getStatus / getCommitFiles
TRPC->>Main: runGitTask (dedupe key, timeout)
Main->>Worker: dispatch/execute git task
Worker->>Git: run git commands
Git-->>Worker: return results
Worker-->>Main: task result
Main-->>TRPC: cached/guarded result
TRPC-->>Renderer: respond with status/files
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Kitenite
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/desktop/src/lib/trpc/routers/projects/projects.ts (1)
286-292:⚠️ Potential issue | 🟠 Major
getRecentsnow drops closed projects from the recent list.
tabOrderis the open-tab marker in this file, not the recency marker. Afterclose()setstabOrdertonull, this query will exclude that project even though its row andlastOpenedAtare intentionally preserved. That means recently closed projects disappear from "recents" and can't be reopened from that list anymore.If the caller needs only open projects, please add a separate query for that instead of narrowing
getRecents.Suggested fix
getRecents: publicProcedure.query((): Project[] => { return localDb .select() .from(projects) - .where(isNotNull(projects.tabOrder)) .orderBy(desc(projects.lastOpenedAt)) .all(); }),Also remove
isNotNullfrom thedrizzle-ormimport on Line 13.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/projects/projects.ts` around lines 286 - 292, getRecents currently filters out rows with tabOrder === null which removes recently closed projects; remove the .where(isNotNull(projects.tabOrder)) clause from the getRecents query so it orders by projects.lastOpenedAt and returns recent projects regardless of open/closed state, and if you need an open-only list add a new procedure (e.g., getOpenProjects) that explicitly filters on projects.tabOrder; also remove isNotNull from the drizzle-orm import since it will no longer be used.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/ChatMastraInterface.tsx (1)
434-446:⚠️ Potential issue | 🟡 MinorRecord the sent image count, not the pre-conversion file count.
attachment_countis now based onfiles.length, but the request payload only sendsimages. After this images-only revert, those values can diverge, so PostHog will over-report attachments for turns where conversion drops anything.Proposed fix
captureChatEvent("chat_message_sent", { session_id: targetSessionId, model_id: activeModel?.id ?? null, mention_count: 0, - attachment_count: files.length, + attachment_count: images.length, is_slash_command: isSlashCommand, message_length: text.length, turn_number: (messages?.length ?? 0) + 1, });Also applies to: 487-493
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/ChatMastraInterface.tsx` around lines 434 - 446, The PostHog event currently records attachment_count using files.length which can differ from the actual images sent after conversion; update the telemetry to record the number of images actually included in the payload (i.e., images.length) instead of files.length. Locate where sendInput is built (payload.content and images via toMastraImages) and where attachment_count is set (also the duplicate block around the 487-493 region) and change those telemetry/event fields to use images.length (or 0 when images is empty) so the recorded attachment_count matches the actual sent images.
🧹 Nitpick comments (9)
apps/desktop/src/main/index.ts (1)
42-50: Consider reusingIS_DEVin thebefore-quithandler.The dev workspace name logic looks correct. However, line 169 duplicates this same check with
const isDev = process.env.NODE_ENV === "development". SinceIS_DEVis already defined at module scope, you could reuse it there for consistency.♻️ Suggested consolidation
app.on("before-quit", async (event) => { if (isQuitting) return; - const isDev = process.env.NODE_ENV === "development"; const shouldConfirm = - !skipConfirmation && !isDev && getConfirmOnQuitSetting(); + !skipConfirmation && !IS_DEV && getConfirmOnQuitSetting();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/index.ts` around lines 42 - 50, The before-quit handler duplicates the development check by defining a new const isDev = process.env.NODE_ENV === "development"; replace that duplicate with the module-scoped IS_DEV constant to keep the environment check consistent; locate the before-quit handler (app.on("before-quit", ...) or similar) and remove the local isDev declaration, using IS_DEV wherever isDev was referenced, and ensure no other local shadowing remains.apps/desktop/src/renderer/stores/hotkeys/store.ts (1)
328-330: Consider adding a JSDoc@deprecatedannotation for better tooling support.The inline comment explains the reasoning well, but adding a JSDoc annotation would enable IDE warnings for callers still passing this parameter.
📝 Suggested improvement
+/** + * `@param` _deps - `@deprecated` Callback refs now keep handlers fresh without listener re-registration. This parameter is ignored. + */ export function useAppHotkey( id: HotkeyId, callback: (event: KeyboardEvent, handler: unknown) => void, options?: { enabled?: boolean; preventDefault?: boolean }, - // Deprecated: callback refs keep handlers fresh without listener re-registration. _deps: unknown[] = [], ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/stores/hotkeys/store.ts` around lines 328 - 330, Add a JSDoc `@deprecated` annotation for the _deps parameter so IDEs warn callers; update the function or method JSDoc to include a `@param` {unknown[]} _deps description followed by `@deprecated` explaining it’s unused (and keep the inline comment), and reference the parameter name _deps in that JSDoc so tooling recognizes the deprecation.apps/desktop/src/renderer/lib/persistent-hash-history/persistent-hash-history.test.ts (1)
242-259: Assert the exact clamp result here.
toBeGreaterThanOrEqual(0)would still pass if this regressed to an off-by-one like1. Since this case is specifically exercising the dropped-region clamp, assert0to lock the behavior down.Proposed test tightening
- expect(stored.index).toBeGreaterThanOrEqual(0); + expect(stored.index).toBe(0);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/lib/persistent-hash-history/persistent-hash-history.test.ts` around lines 242 - 259, Update the assertion in the "stores non-negative cappedIndex when current position is in the dropped portion" test: replace the loose check using expect(stored.index).toBeGreaterThanOrEqual(0) with an exact equality expect(stored.index).toBe(0) so the test asserts the clamp result precisely; the change should be applied in the test that uses createPersistentHashHistory(), history.go(-105), and reads stored via storage.get("router-history").apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsx (1)
15-15: Use import alias instead of deep relative path.This 7-level deep relative import is fragile and hard to maintain. As per coding guidelines: "Use alias as defined in
tsconfig.jsonwhen possible."♻️ Suggested refactor
-import { getRelativeTime } from "../../../../../../../WorkspacesListView/utils"; +import { getRelativeTime } from "@/renderer/screens/main/components/WorkspacesListView/utils";Adjust the alias path based on your actual
tsconfig.jsonpaths configuration.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsx` at line 15, The import in SessionSelector.tsx uses a fragile deep relative path for getRelativeTime; replace the "../../../../../../../WorkspacesListView/utils" import with the TypeScript path alias defined in your tsconfig.json (e.g., the alias that maps to the WorkspacesListView module) so the line importing getRelativeTime uses that alias instead; verify the alias exists in tsconfig.json paths and update any other occurrences if needed, then run a quick build/TS check to ensure no resolution errors.apps/desktop/src/lib/trpc/routers/workspaces/utils/ai-name.ts (1)
63-77: Consider adding timeout protection for resilience.The async call to
generateTitleWithModelhas no timeout. If a provider's API becomes unresponsive, this could block indefinitely. For a non-critical operation like workspace naming, you might want to add a timeout wrapper to improve reliability.💡 Example timeout wrapper
const TITLE_GENERATION_TIMEOUT_MS = 10_000; async function withTimeout<T>( promise: Promise<T>, ms: number, ): Promise<T> { const timeout = new Promise<never>((_, reject) => setTimeout(() => reject(new Error("Timeout")), ms), ); return Promise.race([promise, timeout]); } // Usage in the loop: const title = await withTimeout( generateTitleWithModel(prompt, provider.agentId, provider.createModel(apiKey)), TITLE_GENERATION_TIMEOUT_MS, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/ai-name.ts` around lines 63 - 77, The call to generateTitleWithModel can hang if a provider is unresponsive; wrap that call in a timeout helper (e.g., implement withTimeout<T>(promise, ms) that races the promise against a timeout rejection) and use a sensible constant like TITLE_GENERATION_TIMEOUT_MS (e.g., 10_000) when invoking generateTitleWithModel(provider.agentId, provider.createModel(apiKey)). Replace the direct await with await withTimeout(generateTitleWithModel(...), TITLE_GENERATION_TIMEOUT_MS) and keep the existing try/catch to log failures (including timeout errors) for provider.name so the loop remains resilient.apps/desktop/src/renderer/screens/main/components/ResizablePanel/ResizablePanel.tsx (1)
49-57: Skip no-op width emissions once the value is already clamped.If the pointer keeps moving after the panel hits
minWidthormaxWidth, this still callsonWidthChangeevery frame with the same width. A small equality guard here would avoid unnecessary parent updates during long drags.Also applies to: 78-87
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/ResizablePanel/ResizablePanel.tsx` around lines 49 - 57, The flushPendingWidth function currently always calls onWidthChange with pendingWidth even when that value equals the last emitted/clamped width; add a simple equality guard by storing the last-emitted width in a ref (e.g., lastEmittedWidthRef) and only call onWidthChange(pendingWidth) when pendingWidth !== lastEmittedWidthRef.current, then update lastEmittedWidthRef.current = pendingWidth; apply the same guard to the other width-emission path referenced around the 78-87 block so identical repeated values are not emitted.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AssistantMessage/AssistantMessage.tsx (1)
40-48: KeepImagePartout of this component file.
AssistantMessage.tsxnow contains a second React component undercomponents/**. Since the image markup is only used once, either inline it at the callsite or move it into its ownImagePart/ImagePart.tsxfolder. As per coding guidelines,**/{components,pages}/**/*.{tsx,ts}: No multi-component files - maintain one component per file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AssistantMessage/AssistantMessage.tsx` around lines 40 - 48, AssistantMessage.tsx currently defines a second React component ImagePart which violates the one-component-per-file rule; remove ImagePart from AssistantMessage.tsx and either inline its JSX at the callsite or create a new component file (e.g., ImagePart/ImagePart.tsx) that exports the ImagePart function and then import and use it in AssistantMessage.tsx; ensure the new module exports the same signature ({ data, mimeType }: { data: string; mimeType: string }) and update any imports/usages accordingly.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/EmptyTabView.tsx (1)
140-144: Consider usingcn()utility for className consistency.The codebase uses
cn()from@superset/ui/lib/utilselsewhere (e.g., in ResourceConsumption.tsx). Using it here would maintain consistency.♻️ Optional refactor for consistency
+import { cn } from "@superset/ui/lib/utils"; ... <img alt="Superset" - className={`h-8 w-auto select-none ${ - activeTheme?.type === "dark" - ? "opacity-85" - : "brightness-0 opacity-75" - }`} + className={cn( + "h-8 w-auto select-none", + activeTheme?.type === "dark" + ? "opacity-85" + : "brightness-0 opacity-75" + )} draggable={false} src={supersetEmptyStateWordmark} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/EmptyTabView.tsx` around lines 140 - 144, Replace the inline template string used in the JSX element's className with the shared cn() utility for consistency: import cn from the project's utility (if not already imported), then call cn(...) with the static classes ("h-8 w-auto select-none") and a conditional object/expressions that map activeTheme?.type === "dark" to "opacity-85" and the else branch to "brightness-0 opacity-75"; update the JSX element where className={`h-8 w-auto select-none ${ activeTheme?.type === "dark" ? "opacity-85" : "brightness-0 opacity-75" }`} appears so it uses cn(...) instead.apps/desktop/src/main/lib/resource-metrics/index.ts (1)
358-368: Minor: Consider single cache lookup instead of three.The same
workspaceMetaCache.get(workspaceId)is called three times. A single lookup would be marginally cleaner.♻️ Optional refactor
+ const meta = workspaceMetaCache.get(workspaceId); workspaceMetricsList.push({ workspaceId, - projectId: workspaceMetaCache.get(workspaceId)?.projectId ?? "unknown", - projectName: - workspaceMetaCache.get(workspaceId)?.projectName ?? "Unknown Project", - workspaceName: - workspaceMetaCache.get(workspaceId)?.workspaceName ?? "Unknown", + projectId: meta?.projectId ?? "unknown", + projectName: meta?.projectName ?? "Unknown Project", + workspaceName: meta?.workspaceName ?? "Unknown", cpu: wsCpu, memory: wsMemory, sessions: sessionMetrics, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/lib/resource-metrics/index.ts` around lines 358 - 368, The code repeatedly calls workspaceMetaCache.get(workspaceId) when building the object pushed into workspaceMetricsList; replace the three lookups with a single const (e.g., const meta = workspaceMetaCache.get(workspaceId)) and then use meta?.projectId, meta?.projectName, and meta?.workspaceName with the same fallback values when constructing the object for workspaceMetricsList (keep wsCpu, wsMemory, and sessionMetrics unchanged).
🤖 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/desktop/src/renderer/components/MarkdownRenderer/styles/default/default.css`:
- Around line 6-9: The .default-markdown article selector currently removes a
max-width causing prose to span the full pane; restore a readable line length by
adding a max-width (e.g., 65ch or ~700px) to .default-markdown article and keep
margin: 0 auto to center it, allowing the existing table overflow handling to
manage wide elements while improving readability for normal paragraphs.
In `@apps/desktop/src/renderer/env.renderer.ts`:
- Line 28: SKIP_ENV_VALIDATION currently returns rawEnv before zod defaults are
applied, so NEXT_PUBLIC_OUTLIT_KEY's .default("") is bypassed; update either the
construction of rawEnv or the exported env to coalesce the value (e.g., set
rawEnv.NEXT_PUBLIC_OUTLIT_KEY = rawEnv.NEXT_PUBLIC_OUTLIT_KEY ?? "" before
returning when SKIP_ENV_VALIDATION is true, or ensure env.NEXT_PUBLIC_OUTLIT_KEY
= validatedEnv.NEXT_PUBLIC_OUTLIT_KEY ?? "" after validation) so
NEXT_PUBLIC_OUTLIT_KEY can never be undefined in the dev fast path; reference
symbols: SKIP_ENV_VALIDATION, rawEnv, NEXT_PUBLIC_OUTLIT_KEY, and env.
In `@apps/desktop/src/renderer/lib/trpc-storage.ts`:
- Around line 117-125: The local version write (localStorage.setItem) must not
run before the canonical tRPC persistence (config.set) to avoid the version
getting ahead of stored state; change the sequence in the try block so you first
await config.set(parsed.state), then call
localStorage.setItem(`${name}:version`, String(parsed.version)), and only after
both succeed assign lastFlushedValue = valueToFlush; keep both operations inside
the same try/catch so failures in either are handled the same way and reference
the existing symbols parsed, config.set, localStorage.setItem, lastFlushedValue,
valueToFlush, and name to locate the code to change.
- Around line 152-205: The try/catch in getItem must not drop a fresh pending
snapshot when config.get() throws; change the error path so that if config.get()
fails you still read localStorage for getPendingSnapshotKey(name) and its
updatedAt, evaluate freshness against PENDING_SNAPSHOT_TTL_MS, and if fresh call
scheduleImmediateFlush(name, pendingSnapshot) and return the pendingSnapshot,
otherwise clearPendingSnapshot(name) and return null; keep references to
config.get(), getPendingSnapshotKey(name), getPendingSnapshotUpdatedAtKey(name),
PENDING_SNAPSHOT_TTL_MS, scheduleImmediateFlush, and clearPendingSnapshot to
locate and adjust the logic.
In
`@apps/desktop/src/renderer/routes/_authenticated/components/AgentHooks/hooks/useCommandWatcher/useCommandWatcher.ts`:
- Around line 23-26: The watcher starts while the feature flag is still loading
because useFeatureFlagEnabled can return undefined; change the shouldWatch
condition to require an explicit resolved false value (e.g. remoteAgentDisabled
=== false) instead of using !remoteAgentDisabled so the watcher only starts
after the flag resolves to false; update the expression near
useFeatureFlagEnabled(FEATURE_FLAGS.DISABLE_REMOTE_AGENT) so shouldWatch =
!!deviceInfo && !!organizationId && remoteAgentDisabled === false and ensure any
downstream logic that assumes the watcher ran is gated on shouldWatch.
In
`@apps/desktop/src/renderer/screens/main/components/KeywordSearch/useKeywordSearch.ts`:
- Around line 49-64: The debounced value (debouncedQuery) lags after
setQuery("") causing stale searches; create an effectiveQuery that is the empty
string immediately when trimmedQuery is empty, otherwise use debouncedQuery,
then use that effectiveQuery for the search payload and the enabled flag (and
for isDebouncing calculation) instead of debouncedQuery so clear/close actions
take effect immediately; update references to debouncedQuery in
useDebouncedValue usage, isDebouncing, and the
electronTrpc.filesystem.searchKeyword.useQuery call to use effectiveQuery.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/ChatMastraMessageList.tsx`:
- Line 48: The component ChatMastraMessageList currently aliases the prop
activeSubagents to _activeSubagents and never passes it into the
assistant/tool-inline render path, causing live "Subagent activity" to
disappear; fix by either threading activeSubagents through to the child
renderers (pass activeSubagents into AssistantMessage and ToolPreviewMessage
where they render inline activity) or restore the standalone fallback render
block that consumes activeSubagents (the existing fallback around lines 214-240)
so it still renders while a turn is running; update the prop usage in
ChatMastraMessageList (and any local variables like _activeSubagents) to ensure
activeSubagents is forwarded or used by the inline rendering branches.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/utils/toMastraImages/toMastraImages.ts`:
- Around line 8-18: The code currently accepts any base64 data: URI and forwards
file.mediaType into images.push; update the check in toMastraImages (the block
handling file.url/header/data and images.push) to parse the MIME from the
data-URL header (the portion before any ";"), verify it starts with "image/",
and skip/continue if it does not; also use the parsed MIME as the mimeType
passed to images.push instead of trusting file.mediaType so only true image/*
data URLs are included.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/MentionPopover/MentionPopover.tsx`:
- Around line 113-117: The popover shows stale fileResults while a newer search
is pending; update the logic that computes files (which currently uses open,
immediateSearchQuery, and fileResults) to also check isSearchPending (derived
from immediateSearchQuery !== debouncedSearchQuery || isSearchFetching) and
return an empty array when isSearchPending is true, so files becomes empty
during the debounce/fetch window and stale matches cannot be selected.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/ChangesView.tsx`:
- Around line 53-61: The code currently uses status?.branch as the source of
truth for branch invalidation which can be stale when polling is paused; update
the places that pass branch into useBranchSyncInvalidation (and any other checks
around lines using status?.branch) to prefer the active getBranches result from
branchData first (e.g. use branchData?.branch or the branch property on
branchData) and only fall back to status?.branch if branchData is undefined, so
useGitChangesStatus (status) stays as fallback while branchData (getBranches) is
the primary source for gitBranch invalidation.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/hooks/useFileSearch/useFileSearch.ts`:
- Around line 23-47: Introduce an immediate-clear "effectiveQuery" and use it
consistently so hasQuery reflects the active search term: compute effectiveQuery
= trimmedQuery.length > 0 ? debouncedQuery : "" inside useFileSearch, then pass
effectiveQuery as the query to electronTrpc.filesystem.searchFiles.useQuery and
use Boolean(worktreePath) && effectiveQuery.length > 0 for enabled; finally set
hasQuery to effectiveQuery.length > 0 instead of trimmedQuery.length > 0 so the
hook's returned state matches the live/debounced search state.
In `@apps/desktop/src/renderer/stores/tabs/utils.ts`:
- Around line 217-218: The current derivation of fileName in create/open logic
uses options.filePath.split("/") which fails on Windows backslashes; replace
that logic with a reusable basename helper (e.g., add/get a basename function
used elsewhere) that strips both forward and backslashes and falls back to the
original path when empty, and update the fileName assignment (and the
preview-replacement path in store.ts) to call this basename helper instead of
using split("/"). Ensure the helper is named clearly (e.g., basename) and
referenced from the same module in both utils.ts and store.ts so both Windows
and POSIX paths produce the correct pane/tab title.
In
`@packages/chat-mastra/src/client/hooks/use-mastra-chat-display/use-mastra-chat-display.ts`:
- Around line 135-153: The current reconciliation only matches optimistic sends
by plain text (using optimisticTextRef) and should instead compare a full
payload signature so image-only or caption+image messages aren't cleared by
unrelated text matches; update the optimistic send creation (the code that sets
optimistic state around payload.content in the same file) to compute and store a
stable signature of the full message payload (e.g., a deterministic
serialization of payload.content) and replace optimisticTextRef with an
optimisticPayloadSignatureRef; then change the useEffect that currently inspects
historicalMessages to look for a message whose serialized payload/content
signature equals that stored signature and only then call
setOptimisticUserMessage(null) and clear optimisticPayloadSignatureRef. Ensure
both the reconciliation useEffect and the optimistic-state creation paths (the
block that sets optimistic state when payload.content is non-empty) use the
exact same signature function.
In `@packages/chat-mastra/src/server/trpc/zod.ts`:
- Around line 34-38: The images schema currently allows any MIME type because
mimeType is z.string(); update the zod validator for the images array (the
images: z.array(z.object({...}) schema used for payload.images) to only accept
image MIME types — either replace mimeType: z.string() with a pattern check that
matches image/* (e.g., a regex like /^image\/.+$/) or use an explicit allowlist
of supported image MIME types (e.g., a z.enum of allowed values); keep the
validator on the z.object inside images so callers cannot submit non-image types
into payload.images.
---
Outside diff comments:
In `@apps/desktop/src/lib/trpc/routers/projects/projects.ts`:
- Around line 286-292: getRecents currently filters out rows with tabOrder ===
null which removes recently closed projects; remove the
.where(isNotNull(projects.tabOrder)) clause from the getRecents query so it
orders by projects.lastOpenedAt and returns recent projects regardless of
open/closed state, and if you need an open-only list add a new procedure (e.g.,
getOpenProjects) that explicitly filters on projects.tabOrder; also remove
isNotNull from the drizzle-orm import since it will no longer be used.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/ChatMastraInterface.tsx`:
- Around line 434-446: The PostHog event currently records attachment_count
using files.length which can differ from the actual images sent after
conversion; update the telemetry to record the number of images actually
included in the payload (i.e., images.length) instead of files.length. Locate
where sendInput is built (payload.content and images via toMastraImages) and
where attachment_count is set (also the duplicate block around the 487-493
region) and change those telemetry/event fields to use images.length (or 0 when
images is empty) so the recorded attachment_count matches the actual sent
images.
---
Nitpick comments:
In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/ai-name.ts`:
- Around line 63-77: The call to generateTitleWithModel can hang if a provider
is unresponsive; wrap that call in a timeout helper (e.g., implement
withTimeout<T>(promise, ms) that races the promise against a timeout rejection)
and use a sensible constant like TITLE_GENERATION_TIMEOUT_MS (e.g., 10_000) when
invoking generateTitleWithModel(provider.agentId, provider.createModel(apiKey)).
Replace the direct await with await withTimeout(generateTitleWithModel(...),
TITLE_GENERATION_TIMEOUT_MS) and keep the existing try/catch to log failures
(including timeout errors) for provider.name so the loop remains resilient.
In `@apps/desktop/src/main/index.ts`:
- Around line 42-50: The before-quit handler duplicates the development check by
defining a new const isDev = process.env.NODE_ENV === "development"; replace
that duplicate with the module-scoped IS_DEV constant to keep the environment
check consistent; locate the before-quit handler (app.on("before-quit", ...) or
similar) and remove the local isDev declaration, using IS_DEV wherever isDev was
referenced, and ensure no other local shadowing remains.
In `@apps/desktop/src/main/lib/resource-metrics/index.ts`:
- Around line 358-368: The code repeatedly calls
workspaceMetaCache.get(workspaceId) when building the object pushed into
workspaceMetricsList; replace the three lookups with a single const (e.g., const
meta = workspaceMetaCache.get(workspaceId)) and then use meta?.projectId,
meta?.projectName, and meta?.workspaceName with the same fallback values when
constructing the object for workspaceMetricsList (keep wsCpu, wsMemory, and
sessionMetrics unchanged).
In
`@apps/desktop/src/renderer/lib/persistent-hash-history/persistent-hash-history.test.ts`:
- Around line 242-259: Update the assertion in the "stores non-negative
cappedIndex when current position is in the dropped portion" test: replace the
loose check using expect(stored.index).toBeGreaterThanOrEqual(0) with an exact
equality expect(stored.index).toBe(0) so the test asserts the clamp result
precisely; the change should be applied in the test that uses
createPersistentHashHistory(), history.go(-105), and reads stored via
storage.get("router-history").
In
`@apps/desktop/src/renderer/screens/main/components/ResizablePanel/ResizablePanel.tsx`:
- Around line 49-57: The flushPendingWidth function currently always calls
onWidthChange with pendingWidth even when that value equals the last
emitted/clamped width; add a simple equality guard by storing the last-emitted
width in a ref (e.g., lastEmittedWidthRef) and only call
onWidthChange(pendingWidth) when pendingWidth !== lastEmittedWidthRef.current,
then update lastEmittedWidthRef.current = pendingWidth; apply the same guard to
the other width-emission path referenced around the 78-87 block so identical
repeated values are not emitted.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/EmptyTabView.tsx`:
- Around line 140-144: Replace the inline template string used in the JSX
element's className with the shared cn() utility for consistency: import cn from
the project's utility (if not already imported), then call cn(...) with the
static classes ("h-8 w-auto select-none") and a conditional object/expressions
that map activeTheme?.type === "dark" to "opacity-85" and the else branch to
"brightness-0 opacity-75"; update the JSX element where className={`h-8 w-auto
select-none ${ activeTheme?.type === "dark" ? "opacity-85" : "brightness-0
opacity-75" }`} appears so it uses cn(...) instead.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AssistantMessage/AssistantMessage.tsx`:
- Around line 40-48: AssistantMessage.tsx currently defines a second React
component ImagePart which violates the one-component-per-file rule; remove
ImagePart from AssistantMessage.tsx and either inline its JSX at the callsite or
create a new component file (e.g., ImagePart/ImagePart.tsx) that exports the
ImagePart function and then import and use it in AssistantMessage.tsx; ensure
the new module exports the same signature ({ data, mimeType }: { data: string;
mimeType: string }) and update any imports/usages accordingly.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsx`:
- Line 15: The import in SessionSelector.tsx uses a fragile deep relative path
for getRelativeTime; replace the "../../../../../../../WorkspacesListView/utils"
import with the TypeScript path alias defined in your tsconfig.json (e.g., the
alias that maps to the WorkspacesListView module) so the line importing
getRelativeTime uses that alias instead; verify the alias exists in
tsconfig.json paths and update any other occurrences if needed, then run a quick
build/TS check to ensure no resolution errors.
In `@apps/desktop/src/renderer/stores/hotkeys/store.ts`:
- Around line 328-330: Add a JSDoc `@deprecated` annotation for the _deps
parameter so IDEs warn callers; update the function or method JSDoc to include a
`@param` {unknown[]} _deps description followed by `@deprecated` explaining it’s
unused (and keep the inline comment), and reference the parameter name _deps in
that JSDoc so tooling recognizes the deprecation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4d9b0416-b90c-4b5e-b0c1-969a17206e11
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (71)
apps/desktop/electron.vite.config.tsapps/desktop/package.jsonapps/desktop/src/lib/trpc/routers/changes/branches.tsapps/desktop/src/lib/trpc/routers/changes/status.tsapps/desktop/src/lib/trpc/routers/external/helpers.test.tsapps/desktop/src/lib/trpc/routers/external/helpers.tsapps/desktop/src/lib/trpc/routers/projects/projects.tsapps/desktop/src/lib/trpc/routers/resource-metrics.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/ai-name.tsapps/desktop/src/main/env.main.tsapps/desktop/src/main/index.tsapps/desktop/src/main/lib/resource-metrics/index.tsapps/desktop/src/renderer/components/HotkeyMenuShortcut/HotkeyMenuShortcut.tsxapps/desktop/src/renderer/components/MarkdownRenderer/styles/default/config.tsxapps/desktop/src/renderer/components/MarkdownRenderer/styles/default/default.cssapps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsxapps/desktop/src/renderer/env.renderer.tsapps/desktop/src/renderer/lib/persistent-hash-history/persistent-hash-history.test.tsapps/desktop/src/renderer/lib/persistent-hash-history/persistent-hash-history.tsapps/desktop/src/renderer/lib/sentry.tsapps/desktop/src/renderer/lib/trpc-storage.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/ResourceConsumption/ResourceConsumption.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/page.tsxapps/desktop/src/renderer/routes/_authenticated/components/AgentHooks/hooks/useCommandWatcher/useCommandWatcher.tsapps/desktop/src/renderer/screens/main/components/KeywordSearch/useKeywordSearch.tsapps/desktop/src/renderer/screens/main/components/ResizablePanel/ResizablePanel.tsxapps/desktop/src/renderer/screens/main/components/SidebarControl/SidebarControl.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/hooks/usePortsData.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/ChangesContent.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/EmptyTabView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/ChatMastraInterface.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/ChatMastraMessageList.test.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/ChatMastraMessageList.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AssistantMessage/AssistantMessage.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AttachmentChip/AttachmentChip.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AttachmentChip/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/PendingPlanApprovalMessage/PendingPlanApprovalMessage.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/SubagentExecutionMessage/SubagentExecutionMessage.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/SubagentExecutionMessage/utils/toSubagentViewModels.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/hooks/useOptimisticUpload/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/hooks/useOptimisticUpload/useOptimisticUpload.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/utils/optimisticUserMessage/optimisticUserMessage.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/utils/sendMessage/sendMessage.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/utils/toMastraImages/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/utils/toMastraImages/toMastraImages.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/utils/uploadFiles/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/utils/uploadFiles/uploadFiles.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/ChatInputFooter/ChatInputFooter.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/ChatInputFooter/components/ChatComposerControls/ChatComposerControls.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/MentionPopover/MentionPopover.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/ChangesView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/hooks/useFileSearch/useFileSearch.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceLayout/WorkspaceLayout.tsxapps/desktop/src/renderer/screens/main/components/WorkspacesListView/utils.tsapps/desktop/src/renderer/screens/main/hooks/useGitChangesStatus/useGitChangesStatus.tsapps/desktop/src/renderer/stores/hotkeys/store.tsapps/desktop/src/renderer/stores/sidebar-state.tsapps/desktop/src/renderer/stores/tabs/store.tsapps/desktop/src/renderer/stores/tabs/types.tsapps/desktop/src/renderer/stores/tabs/utils.tspackage.jsonpackages/chat-mastra/src/client/hooks/use-mastra-chat-display/use-mastra-chat-display.tspackages/chat-mastra/src/server/trpc/zod.tspackages/ui/src/components/ai-elements/prompt-input.tsx
💤 Files with no reviewable changes (8)
- apps/desktop/src/renderer/stores/tabs/types.ts
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AttachmentChip/index.ts
- apps/desktop/src/renderer/lib/sentry.ts
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/utils/uploadFiles/index.ts
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/hooks/useOptimisticUpload/index.ts
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AttachmentChip/AttachmentChip.tsx
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/utils/uploadFiles/uploadFiles.ts
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/hooks/useOptimisticUpload/useOptimisticUpload.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AssistantMessage/AssistantMessage.tsx (2)
42-50: ExtractImagePartto its own file to comply with component structure guidelines.This file now contains two components (
ImagePartandAssistantMessage), which violates the single-component-per-file guideline. Consider movingImagePartto its own folder.📁 Suggested structure
Create a new component folder:
components/ImagePart/ ├── ImagePart.tsx └── index.tsImagePart.tsx:
export function ImagePart({ data, mimeType }: { data: string; mimeType: string }) { return ( <img src={`data:${mimeType};base64,${data}`} alt="Attached" className="max-h-48 rounded-lg object-contain" /> ); }index.ts:
export { ImagePart } from "./ImagePart";As per coding guidelines:
**/{components,pages}/**/*.{tsx,ts}: No multi-component files - maintain one component per file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AssistantMessage/AssistantMessage.tsx` around lines 42 - 50, The file currently defines two components; extract the ImagePart component into its own folder and re-export it so the file adheres to the single-component-per-file guideline: create components/ImagePart/ImagePart.tsx containing the ImagePart function (signature: ImagePart({ data, mimeType }: { data: string; mimeType: string })) and an index.ts that exports it, then update AssistantMessage.tsx to import { ImagePart } from the new folder and remove the local ImagePart definition.
201-224: Consider adding a clarifying comment for the different image data formats.The two rendering paths expect different data formats:
ImagePartconstructs a data URL from raw base64, while the direct<img>assumesdatais already a complete URL. A brief comment explaining when each format is expected would help maintainability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AssistantMessage/AssistantMessage.tsx` around lines 201 - 224, The code has two distinct image-rendering paths with different expected data formats but no comment: the branch that renders ImagePart (check part.type === "image" && "mimeType" in part && !rawPart.mediaType) expects raw base64 data that ImagePart will turn into a data URL, while the branch using an <img> inside the button (mediaType.startsWith("image/")) expects data to already be a complete URL; add a short clarifying comment above these branches mentioning these expectations and referencing ImagePart, mediaType, rawPart.mediaType and handleAttachmentClick so future readers know when to supply base64 vs full URLs.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx (1)
48-63: Address the type definition gap for message content parts.The code casts
partandrawPartto custom object shapes to access"file"type and properties likedata,filename, andmimeType. This indicates thatMastraMessagePart(derived fromMastraMessage["content"][number]) does not include all runtime content part shapes in its type definition.The same pattern appears in the core hook (
packages/chat-mastra/src/client/hooks/use-mastra-chat-display/use-mastra-chat-display.ts:89), confirming the type gap exists upstream. Consider extending the message content type union in@superset/chat-mastrato include"file"and related properties, or define a local augmented type to improve type safety.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx` around lines 48 - 63, The message content parts lack a proper "file" variant in the type union, so code in UserMessage.tsx (handling message.content and MastraMessagePart) and the upstream hook use-mastra-chat-display are using unsafe casts; fix by adding a proper "file" content variant to the MastraMessagePart / MastraMessage["content"][number] union in the `@superset/chat-mastra` types (include type: "file" and properties data?: string, filename?: string, mediaType?: string, mimeType?: string), then update UserMessage.tsx and the hook to import and use that typed variant instead of casting rawPart, removing the ad-hoc casts and narrowing on part.type === "file" to safely access file properties.
🤖 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/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/ResourceConsumption/ResourceConsumption.tsx`:
- Around line 114-117: The call getUsageSeverity(totalUsage, totalUsage, {
includeShare: false }) in ResourceConsumption.tsx passes the same value for
values and totals unnecessarily; either (A) if you intended to evaluate absolute
totals without share logic, remove/omit the second argument or pass
null/undefined so getUsageSeverity only considers values when
includeShare:false, or (B) if you intended to compare against a separate
total/limit, replace the second argument with the correct totals variable used
elsewhere (match other calls to getUsageSeverity) so totalSeverity reflects
comparison against the proper threshold; update the call that computes
totalSeverity and then use getUsageClasses(totalSeverity) as before.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AssistantMessage/AssistantMessage.tsx`:
- Around line 212-224: The image button in AssistantMessage.tsx is missing an
accessible label for screen readers; update the button that wraps the <img> (the
element using onClick={() => handleAttachmentClick(data, rawPart.filename)}) to
include a descriptive aria-label (for example "Open attachment
{rawPart.filename}" or "View image {rawPart.filename}") that uses
rawPart.filename or a fallback like "Generated image", so assistive tech conveys
the button's purpose; keep the existing img alt text and existing onClick
handler (handleAttachmentClick) intact.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/MessageScrollbackRail/MessageScrollbackRail.tsx`:
- Around line 43-46: The scrollback preview is still counting "file"
attachments; update the filter in MessageScrollbackRail by removing the file
branch so only images are considered: change the attachmentCount calculation
that iterates over message.content (MastraMessagePart) to only check part.type
=== "image" (handle optional type safely if needed) so file-only messages no
longer produce scrollback previews.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx`:
- Line 82: The src attribute in the image/data URL is inconsistently reading
part.data instead of the already-extracted data variable; update the JSX in
UserMessage (the src={`data:${part.mimeType};base64,${part.data}`} usage) to use
the extracted data variable (e.g., src={`data:${part.mimeType};base64,${data}`})
so that the component consistently uses the value pulled from rawPart.data.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/hooks/useOptimisticUpload/useOptimisticUpload.ts`:
- Around line 38-39: The optimistic-upload cache must be scoped to the active
session/attachment set: capture the current sessionId at start of
useOptimisticUpload (or add a sessionVersionRef) and use it to namespace or
version entriesRef and inflightRef (e.g., include sessionId in cache keys or
reset the refs when sessionId changes); additionally, in the async promise
handlers that currently write file.id back into entriesRef (the handlers around
Lines 54-80) and in the cleanup logic (Lines 84-95), first verify the captured
sessionId/version still matches the current sessionId before mutating
entriesRef/inflightRef, and ignore completions for attachments that are no
longer present so stale file ids are not reinserted into entries returned by
entriesRef.current.
- Around line 54-63: The promise resolution handler for uploadFiles currently
treats a falsy uploaded value as success; update the .then(([uploaded]) => { ...
}) block (the inflightRef.current.delete(file.id) / setEntries(...) logic) to
check if uploaded is truthy and, if not, mark the entry as failed (set error to
a meaningful value, uploaded: null, uploading: false) or invoke the existing
.catch() failure path instead of recording a successful no-op entry; also mirror
this change in the getUploadedFiles logic (the function handling
ready/attachment selection) so it does not set ready: true or drop an attachment
when uploaded is missing—use the same entry keys and status updates that the
catch path uses (refer to inflightRef, setEntries, uploadFiles resolution
handler, and getUploadedFiles).
---
Nitpick comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AssistantMessage/AssistantMessage.tsx`:
- Around line 42-50: The file currently defines two components; extract the
ImagePart component into its own folder and re-export it so the file adheres to
the single-component-per-file guideline: create
components/ImagePart/ImagePart.tsx containing the ImagePart function (signature:
ImagePart({ data, mimeType }: { data: string; mimeType: string })) and an
index.ts that exports it, then update AssistantMessage.tsx to import { ImagePart
} from the new folder and remove the local ImagePart definition.
- Around line 201-224: The code has two distinct image-rendering paths with
different expected data formats but no comment: the branch that renders
ImagePart (check part.type === "image" && "mimeType" in part &&
!rawPart.mediaType) expects raw base64 data that ImagePart will turn into a data
URL, while the branch using an <img> inside the button
(mediaType.startsWith("image/")) expects data to already be a complete URL; add
a short clarifying comment above these branches mentioning these expectations
and referencing ImagePart, mediaType, rawPart.mediaType and
handleAttachmentClick so future readers know when to supply base64 vs full URLs.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx`:
- Around line 48-63: The message content parts lack a proper "file" variant in
the type union, so code in UserMessage.tsx (handling message.content and
MastraMessagePart) and the upstream hook use-mastra-chat-display are using
unsafe casts; fix by adding a proper "file" content variant to the
MastraMessagePart / MastraMessage["content"][number] union in the
`@superset/chat-mastra` types (include type: "file" and properties data?: string,
filename?: string, mediaType?: string, mimeType?: string), then update
UserMessage.tsx and the hook to import and use that typed variant instead of
casting rawPart, removing the ad-hoc casts and narrowing on part.type === "file"
to safely access file properties.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 24681c72-148b-4f55-91d7-92f0946795a9
📒 Files selected for processing (9)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/ResourceConsumption/ResourceConsumption.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/ChatMastraInterface.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AssistantMessage/AssistantMessage.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/MessageScrollbackRail/MessageScrollbackRail.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/hooks/useOptimisticUpload/useOptimisticUpload.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/utils/optimisticUserMessage/optimisticUserMessage.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/utils/sendMessage/sendMessage.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/utils/uploadFiles/uploadFiles.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/utils/uploadFiles/uploadFiles.ts
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/utils/optimisticUserMessage/optimisticUserMessage.ts
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/utils/sendMessage/sendMessage.ts
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/ChatMastraInterface.tsx
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/ChatMastraInterface.tsx (1)
65-99:⚠️ Potential issue | 🟠 MajorThis still preserves the generic file-upload flow.
mediaTypeis copied through unchanged in both branches,uploadFilesstill runs for pending attachments, and those files are still forwarded intosendInput.payload.files. Since there is no image-only guard anywhere in this path, non-image attachments can still go through this pane, so the revert does not actually restore the previous images-only behavior.Also applies to: 519-563, 868-886
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/ChatMastraInterface.tsx` around lines 65 - 99, handleSend currently forwards attachments unchanged (via getUploadedFiles and the message.files branch) allowing non-image files into sendInput.payload.files; update handleSend (and the related branches around lines referenced) to filter attachments to image-only before mapping: check file.mediaType startsWith('image/') (or match allowed image MIME list) and drop non-image entries so upload/optimistic upload (useOptimisticUpload/getUploadedFiles) and onSend only receive image files; also ensure the branch that sets uploaded: false for pending attachments similarly filters out non-image files so uploadFiles is not invoked for them.
🧹 Nitpick comments (2)
packages/chat-mastra/src/client/hooks/use-mastra-chat-display/use-mastra-chat-display.ts (1)
347-347: Consider removinghistoricalMessagesfrom the dependency array.Including
historicalMessagescauses thecommandsobject to be recreated on every message poll, generating new function references. SincehistoricalMessagesis only used to snapshot the file count at send time (line 234), consider using a ref to read the latest value instead:♻️ Suggested approach
+const historicalMessagesRef = useRef<ListMessagesOutput>([]); +historicalMessagesRef.current = historicalMessages; const commands = useMemo( () => ({ sendMessage: async (...) => { // ... if (!text) { fileMessageCountAtSendRef.current = - countFileMessages(historicalMessages); + countFileMessages(historicalMessagesRef.current); } // ... }, // ... }), - [cwd, historicalMessages, sessionId, utils], + [cwd, sessionId, utils], );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/chat-mastra/src/client/hooks/use-mastra-chat-display/use-mastra-chat-display.ts` at line 347, Remove historicalMessages from the dependency array that recreates the commands object; instead create a ref (e.g., const historicalMessagesRef = useRef(historicalMessages)) and keep it updated (useEffect(() => { historicalMessagesRef.current = historicalMessages }, [historicalMessages])). Update the send handler (the function that snapshots file count when sending) and any place inside commands that reads historicalMessages to read historicalMessagesRef.current so commands can be memoized without historicalMessages in its deps array.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/ChatMastraInterface.tsx (1)
51-121: Please extractMastraUploadFooterinto its own component file.
ChatMastraInterface.tsxis already carrying most of the session/send orchestration, and adding another component here makes the file harder to navigate. PullingMastraUploadFooterinto its own folder would keep this surface much easier to maintain.As per coding guidelines, "One folder per component with structure:
ComponentName/ComponentName.tsx+index.tsfor barrel export" and "No multi-component files - maintain one component per file".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/ChatMastraInterface.tsx` around lines 51 - 121, Extract the MastraUploadFooter component into its own foldered component by creating a MastraUploadFooter/MastraUploadFooter.tsx and MastraUploadFooter/index.ts barrel export, move the MastraUploadFooter function (including its imports of useProviderAttachments, useOptimisticUpload, ChatInputFooter, PromptInputAttachment, PromptInputMessage types, etc.) into that new file, update ChatMastraInterface.tsx to import MastraUploadFooter from the new barrel, and ensure props and hooks signatures remain unchanged (sessionId, onError, onSend, footerProps) so behavior with getUploadedFiles, entries, isUploading and renderAttachment is preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/ChatMastraInterface.tsx`:
- Around line 65-99: handleSend currently forwards attachments unchanged (via
getUploadedFiles and the message.files branch) allowing non-image files into
sendInput.payload.files; update handleSend (and the related branches around
lines referenced) to filter attachments to image-only before mapping: check
file.mediaType startsWith('image/') (or match allowed image MIME list) and drop
non-image entries so upload/optimistic upload
(useOptimisticUpload/getUploadedFiles) and onSend only receive image files; also
ensure the branch that sets uploaded: false for pending attachments similarly
filters out non-image files so uploadFiles is not invoked for them.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/ChatMastraInterface.tsx`:
- Around line 51-121: Extract the MastraUploadFooter component into its own
foldered component by creating a MastraUploadFooter/MastraUploadFooter.tsx and
MastraUploadFooter/index.ts barrel export, move the MastraUploadFooter function
(including its imports of useProviderAttachments, useOptimisticUpload,
ChatInputFooter, PromptInputAttachment, PromptInputMessage types, etc.) into
that new file, update ChatMastraInterface.tsx to import MastraUploadFooter from
the new barrel, and ensure props and hooks signatures remain unchanged
(sessionId, onError, onSend, footerProps) so behavior with getUploadedFiles,
entries, isUploading and renderAttachment is preserved.
In
`@packages/chat-mastra/src/client/hooks/use-mastra-chat-display/use-mastra-chat-display.ts`:
- Line 347: Remove historicalMessages from the dependency array that recreates
the commands object; instead create a ref (e.g., const historicalMessagesRef =
useRef(historicalMessages)) and keep it updated (useEffect(() => {
historicalMessagesRef.current = historicalMessages }, [historicalMessages])).
Update the send handler (the function that snapshots file count when sending)
and any place inside commands that reads historicalMessages to read
historicalMessagesRef.current so commands can be memoized without
historicalMessages in its deps array.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 31c96985-cbe2-439f-8dc4-3a215422d48e
📒 Files selected for processing (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/ChatMastraInterface.tsxpackages/chat-mastra/src/client/hooks/use-mastra-chat-display/use-mastra-chat-display.ts
Contributor
There was a problem hiding this comment.
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/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx`:
- Around line 127-135: The attachment click currently passes raw base64 data to
openAttachment which becomes addFileViewerPane.filePath, causing
isImageFile/isRemote checks to fail; update the AttachmentChip onClick (in
UserMessage.tsx) to pass a proper data URL instead of raw base64 so the file
viewer and useFileContent detect images correctly (construct
"data:{mediaType};base64,{data}" when calling openAttachment with
rawPart.filename); ensure openAttachment/addFileViewerPane and downstream checks
still receive filename as before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 97db8e8b-843d-4390-8394-668e8495012e
📒 Files selected for processing (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/hooks/useOptimisticUpload/useOptimisticUpload.ts (1)
141-146: Redundant conditional check.After passing the
!entry.uploadedcheck on line 141,entry.uploadedis guaranteed to be truthy. Theif (entry.uploaded)on line 144 is always true and can be simplified.♻️ Suggested simplification
if (entry.error || !entry.uploaded) { return { ready: false, files: [] }; } - if (entry.uploaded) { - files.push(entry.uploaded); - } + files.push(entry.uploaded); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/hooks/useOptimisticUpload/useOptimisticUpload.ts` around lines 141 - 146, The conditional is redundant: after the guard if (entry.error || !entry.uploaded) returns, entry.uploaded is guaranteed truthy, so remove the second if (entry.uploaded) branch and directly push entry.uploaded into files; update the block in useOptimisticUpload where entry is inspected (the guard using entry.error || !entry.uploaded and the subsequent push to files) to simplify the logic and avoid the always-true check.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AssistantMessage/AssistantMessage.tsx (1)
42-50: ExtractImagePartinto its own component file.This helper makes
AssistantMessage.tsxa multi-component file undercomponents/. Moving it to its ownImagePart/ImagePart.tsxkeeps the folder contract consistent and makes the attachment rendering path easier to test in isolation.As per coding guidelines, "
**/{components,pages}/**/*.{ts,tsx}: One folder per component with structure:ComponentName/ComponentName.tsx+index.tsfor barrel export" and "/{components,pages}//*.{tsx,ts}: No multi-component files - maintain one component per file."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AssistantMessage/AssistantMessage.tsx` around lines 42 - 50, Move the ImagePart helper out of AssistantMessage.tsx into its own component folder; create components/ChatMastraMessageList/components/AssistantMessage/ImagePart/ImagePart.tsx that exports the ImagePart functional component (same props: data, mimeType) as the default export, add an index.ts that re-exports it, then remove the ImagePart declaration from AssistantMessage.tsx and update any imports in AssistantMessage.tsx to import ImagePart from './ImagePart' (or the new relative path) so rendering and tests reference the standalone ImagePart component.
🤖 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/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/ChatMastraMessageList.tsx`:
- Around line 153-158: hasConversationContent currently only checks
renderedMessages and interruptedPreview and therefore still shows the
empty-state when transient UI (currentMessage, subagent activity, tool previews,
or pending approval/question blocks) is present; update hasConversationContent
to include those transient signals (e.g., include Boolean(currentMessage),
isSubagentActive, hasToolPreview, hasPendingApprovalOrQuestion) so that
shouldShowConversationLoading and shouldShowEmptyState (which use
hasConversationContent) suppress the empty state while any transient chat
content is visible.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AssistantMessage/AssistantMessage.tsx`:
- Around line 42-50: Move the ImagePart helper out of AssistantMessage.tsx into
its own component folder; create
components/ChatMastraMessageList/components/AssistantMessage/ImagePart/ImagePart.tsx
that exports the ImagePart functional component (same props: data, mimeType) as
the default export, add an index.ts that re-exports it, then remove the
ImagePart declaration from AssistantMessage.tsx and update any imports in
AssistantMessage.tsx to import ImagePart from './ImagePart' (or the new relative
path) so rendering and tests reference the standalone ImagePart component.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/hooks/useOptimisticUpload/useOptimisticUpload.ts`:
- Around line 141-146: The conditional is redundant: after the guard if
(entry.error || !entry.uploaded) returns, entry.uploaded is guaranteed truthy,
so remove the second if (entry.uploaded) branch and directly push entry.uploaded
into files; update the block in useOptimisticUpload where entry is inspected
(the guard using entry.error || !entry.uploaded and the subsequent push to
files) to simplify the logic and avoid the always-true check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c2eb9e6b-0479-492c-bbc8-c14d1d5febed
📒 Files selected for processing (4)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/ChatMastraMessageList.test.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/ChatMastraMessageList.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AssistantMessage/AssistantMessage.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/hooks/useOptimisticUpload/useOptimisticUpload.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/ChatMastraMessageList.test.tsx
Comment on lines
153
to
158
| const hasConversationContent = | ||
| renderedMessages.length > 0 || Boolean(interruptedPreview); | ||
| const shouldShowConversationLoading = | ||
| isConversationLoading && !isAwaitingAssistant && !hasConversationContent; | ||
| const shouldShowEmptyState = | ||
| !shouldShowConversationLoading && !hasConversationContent; |
Contributor
There was a problem hiding this comment.
Suppress the empty state while transient chat content is visible.
hasConversationContent only counts persisted/interrupted messages, so a fresh run can still render "Start a conversation" alongside currentMessage, subagent activity, tool previews, or pending approval/question blocks.
💡 Proposed fix
- const hasConversationContent =
- renderedMessages.length > 0 || Boolean(interruptedPreview);
+ const hasConversationContent =
+ renderedMessages.length > 0 ||
+ Boolean(interruptedPreview) ||
+ Boolean(currentMessage) ||
+ hasSubagentActivity ||
+ shouldShowThinking ||
+ shouldShowToolPreview ||
+ Boolean(pendingApproval) ||
+ Boolean(pendingPlanApproval) ||
+ Boolean(pendingQuestion);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/ChatMastraMessageList.tsx`
around lines 153 - 158, hasConversationContent currently only checks
renderedMessages and interruptedPreview and therefore still shows the
empty-state when transient UI (currentMessage, subagent activity, tool previews,
or pending approval/question blocks) is present; update hasConversationContent
to include those transient signals (e.g., include Boolean(currentMessage),
isSubagentActive, hasToolPreview, hasPendingApprovalOrQuestion) so that
shouldShowConversationLoading and shouldShowEmptyState (which use
hasConversationContent) suppress the empty state while any transient chat
content is visible.
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.
Summary
What This PR Restores
What This PR Intentionally Does Not Restore
These changes were reverted out with #2058, but they were unrelated to the original file-support intent and should stay out of this PR:
Related PRs / Context
Key Follow-up Commits In This PR
Testing