feat(frontend): @agenta/chat — the shared chat engine, composer and approval card - #5870
feat(frontend): @agenta/chat — the shared chat engine, composer and approval card#5870ardaerzin wants to merge 4 commits into
Conversation
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR adds shared agent-chat entry points, attachment upload and preview infrastructure, reusable composer and approval components, transcript error handling, session revalidation, and smoother live text streaming. ChangesShared agent-chat platform
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ChatComposer
participant useComposerAttachments
participant uploadAttachment
participant AgentConversation
User->>ChatComposer: Select or paste files
ChatComposer->>useComposerAttachments: Stage and validate files
useComposerAttachments->>uploadAttachment: Upload multipart attachment
uploadAttachment-->>useComposerAttachments: Return validated attachment reference
useComposerAttachments-->>ChatComposer: Update preview and send gating
User->>ChatComposer: Submit message
ChatComposer->>AgentConversation: Send text and attachment parts
AgentConversation-->>ChatComposer: Stream response and persist transcript
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (14)
web/packages/agenta-playground/src/agentChat.ts (1)
1-7: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReduce new implementation comments to one short line.
The new comments exceed the repository comment limit. Keep longer comments only for a genuinely surprising bug, race, or ordering constraint.
web/packages/agenta-playground/src/agentChat.ts#L1-L7: reduce the entry-point rationale to one short line.web/packages/agenta-playground/src/state/execution/executionHeaders.ts#L3-L22: remove the in-code example and reduce the rationale to one short line.web/packages/agenta-playground/src/state/execution/webWorkerIntegration.ts#L87-L88: use one short re-export comment.web/packages/agenta-chat/src/transport/AgentChatTransport.ts#L207-L220: reduce the stream-smoothing rationale to one short line.web/packages/agenta-chat/src/transport/AgentChatTransport.ts#L232-L233: use one short comment for delta matching.web/packages/agenta-chat/src/model/parts.ts#L58-L60: reduce the performance rationale to one short line.As per coding guidelines, “Keep in-code comments to at most one short line; use longer comments only for genuinely surprising constraints such as bugs, races, or ordering requirements.”
Source: Coding guidelines
web/packages/agenta-chat/src/assets/attachmentRules.ts (1)
146-152: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDerive
isViewablefromkindForTypeto keep one media-type table.The predicate repeats the exact member list of
KIND_TYPES.documentplusimage/. IfKIND_TYPES.documentchanges later, this list silently drifts.♻️ Proposed refactor
-export const isViewable = (mediaType: string): boolean => - mediaType.startsWith("image/") || - mediaType === "application/pdf" || - mediaType.startsWith("text/") || - mediaType === "application/json" +export const isViewable = (mediaType: string): boolean => { + const kind = kindForType(mediaType) + return kind === "image" || kind === "document" +}web/packages/agenta-chat/src/assets/attachmentTransport.ts (1)
47-58: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueUse the caller's limits for the 413 message, not the defaults.
errorForResponsereadsDEFAULT_ATTACHMENT_LIMITS.maxBytes.validateIncomingaccepts a caller-suppliedAttachmentLimits, anduseComposerAttachmentsdocuments that limits will later come from capability gating. If a host narrows the limits, the 413 text will state a size that does not match the enforced client limit.Consider passing the active
AttachmentLimitsintouploadAttachmentand through toerrorForResponse.web/packages/agenta-chat/tests/unit/assets/attachmentRules.test.ts (1)
26-33: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSimplify the narrowed-limits typing.
The test declares
kindswithas constand then spreads it again at the call site to get a mutable array. The later test at Lines 60-63 solves the same problem with an explicit array type. Use one style, for examplekinds: ["image", "audio", "document"] as AttachmentKind[].web/packages/agenta-chat/tests/unit/assets/files.test.ts (1)
158-195: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd a negative case for
attachmentIdForPart.The suite covers the reference-part happy path.
attachmentIdForPartalso guards a missingproviderMetadata, a non-objectagentavalue, and an empty-stringattachmentId. A part withproviderMetadata: {agenta: {attachmentId: ""}}must fall through to the URL-tail branch offilePartName. That branch is currently untested.web/packages/agenta-chat/src/components/index.ts (1)
10-11: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExport the prop types for the two default-exported components.
ApprovalCardandChatComposerre-export their prop types.ComposerAttachmentsandAudioPlayerdo not. Consumers outside the package cannot type a wrapper around them.ComposerAttachmentsPropsalready exists inComposerAttachments.tsx.web/packages/agenta-chat/tests/unit/hooks/useComposerAttachments.test.ts (1)
74-85: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAssert the exact count for the same-tick guard.
Two batches of
Math.ceil(maxCount / 2) + 1files stage exactlymaxCountentries when the guard works.toBeLessThanOrEqualalso passes if the hook stages far fewer files, for example if the second batch is dropped entirely. UsetoBe(DEFAULT_ATTACHMENT_LIMITS.maxCount).web/packages/agenta-chat/src/components/AudioPlayer.tsx (1)
50-67: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd an event or timeout exit path for the duration probe.
currentupdates are skipped whileprobingRef.currentis true, but the probe only waits forfinishProbefrom atimeupdateafterel.currentTime = 1e101. If that seek is clamped or otherwise does not emittimeupdate, this flag remains true and the elapsed-time display stops advancing. Use an additional completion event such asdurationchange, or fall back after a short timeout, so the probe cannot leave the player in a paused timer state.web/packages/agenta-chat/src/model/attachments.ts (1)
36-38: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
typeandsizehave no producer or consumer.
toUploadFileinweb/packages/agenta-chat/src/hooks/useComposerAttachments.tsnever setstypeorsize, andComposerAttachments.tsxderives both fromoriginFileObjonly. The declared fallback for rows whose blob is gone does not work today. Either populate the fields intoUploadFileand read them in the tile renderer, or drop the fields until a serialized path exists.web/packages/agenta-chat/src/hooks/useComposerAttachments.ts (2)
39-45: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the leftover cast.
StagedUpload["originFileObj"]isFile | undefined, so aFileassigns directly. The cast is a shim left from the antdUploadFiletype.♻️ Proposed cleanup
- originFileObj: file as UploadFile["originFileObj"], + originFileObj: file,As per coding guidelines: for workspace packages, "avoid
anyand legacy compatibility shims".Source: Coding guidelines
242-262: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffThe hook returns a new object and new handler identities on every render.
addFiles,removeFile,uploadExtraFiles,bindDropTarget, andclearAttachmentsare recreated each render, and the returned object literal is a new reference each render.ChatComposerpassesaddFilesandremoveFilestraight intoComposerAttachments, so no consumer can memoize on them, and hosts that receive the wholeattachmentsobject re-render on every parent render.Wrap the handlers in
useCallbackand the result inuseMemo.As per coding guidelines: "Minimize React re-renders with
useMemo,useCallback, andReact.memowhere appropriate; avoid unstable inline functions and objects".Source: Coding guidelines
web/packages/agenta-chat/src/components/ComposerAttachments.tsx (1)
69-73: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace the raw
rgba()colors with Tailwind color utilities.Lines 71, 96, and 98 hard-code
rgba(0,0,0,0.6),rgba(0,0,0,0.4), andrgba(255,255,255,0.3). Tailwind opacity utilities give the same result inside the token system.♻️ Proposed change
- ? `absolute right-1 top-1 flex h-5 w-5 cursor-pointer items-center justify-center rounded-full border-0 bg-[rgba(0,0,0,0.6)] text-white transition-opacity ${persistent ? "" : "opacity-0 group-hover:opacity-100 focus-visible:opacity-100"}` + ? `absolute right-1 top-1 flex h-5 w-5 cursor-pointer items-center justify-center rounded-full border-0 bg-black/60 text-white transition-opacity ${persistent ? "" : "opacity-0 group-hover:opacity-100 focus-visible:opacity-100"}`- <div className="pointer-events-none absolute inset-0 rounded-lg bg-[rgba(0,0,0,0.4)]" /> + <div className="pointer-events-none absolute inset-0 rounded-lg bg-black/40" /> <div className="pointer-events-none absolute inset-x-1.5 bottom-1.5 flex items-center gap-1.5"> - <div className="h-1 min-w-0 flex-1 overflow-hidden rounded-full bg-[rgba(255,255,255,0.3)]"> + <div className="h-1 min-w-0 flex-1 overflow-hidden rounded-full bg-white/30">As per coding guidelines: "Consume theme colors through Ant Design semantic tokens, Tailwind color utilities, or supported
var(--ag-color*)variables; do not use raw hex colors or--ag-c-*literals."Also applies to: 96-98
Source: Coding guidelines
web/packages/agenta-chat/tests/unit/hooks/useAttachmentUploads.test.ts (1)
6-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the hook itself.
Both tests exercise pure helpers.
useAttachmentUploadsholds the risky logic: the resume rule at Line 131, abort, retry scheduling, and unmount cleanup.@testing-library/reactandjsdomare already devDependencies, sorenderHookis available. A test that aborts an uploading row and then re-renders would pin the behavior I flagged atweb/packages/agenta-chat/src/hooks/useAttachmentUploads.tsLines 116-145.Also move the
abortassertion out of thesetFilescallback so it runs unconditionally.💚 Proposed change
removeUploadFile("remove-me", abort, (updater) => { - expect(abort).toHaveBeenCalledWith("remove-me") nextFiles = updater(files) }) + expect(abort).toHaveBeenCalledWith("remove-me") expect(nextFiles).toEqual([files[1]])Do you want me to generate the
renderHooktests for the abort and retry paths?web/packages/agenta-chat/src/hooks/useAttachmentUploads.ts (1)
51-53: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove
filesRef.current = filesout of render.Assigning
filesRef.currentin the component body mutates a ref while React renders, which is unsafe under concurrent rendering. React 19’s only render-phase ref write exception is initialization withref.current === null, not later assignments. Move this assignment into an effect ifrunneeds the latest non-emptyfilesarray.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: be24ff99-f5a1-492f-a385-5e25ed3d4cca
📒 Files selected for processing (45)
web/packages/agenta-chat/package.jsonweb/packages/agenta-chat/src/assets/attachmentRules.tsweb/packages/agenta-chat/src/assets/attachmentTransport.tsweb/packages/agenta-chat/src/assets/files.tsweb/packages/agenta-chat/src/assets/index.tsweb/packages/agenta-chat/src/assets/loadSession.tsweb/packages/agenta-chat/src/assets/motion.tsweb/packages/agenta-chat/src/assets/rewind.tsweb/packages/agenta-chat/src/assets/toolFormat.tsweb/packages/agenta-chat/src/assets/trace.tsweb/packages/agenta-chat/src/assets/transcriptToMessages.tsweb/packages/agenta-chat/src/components/ApprovalCard.tsxweb/packages/agenta-chat/src/components/AudioPlayer.tsxweb/packages/agenta-chat/src/components/ChatComposer.tsxweb/packages/agenta-chat/src/components/ComposerAttachments.tsxweb/packages/agenta-chat/src/components/index.tsweb/packages/agenta-chat/src/hooks/index.tsweb/packages/agenta-chat/src/hooks/useAgentChatQueue.tsweb/packages/agenta-chat/src/hooks/useAgentConversation.tsweb/packages/agenta-chat/src/hooks/useAgentModelKeyStatus.tsweb/packages/agenta-chat/src/hooks/useAlwaysAllowTool.tsweb/packages/agenta-chat/src/hooks/useAttachmentUploads.tsweb/packages/agenta-chat/src/hooks/useComposerAttachments.tsweb/packages/agenta-chat/src/model/attachments.tsweb/packages/agenta-chat/src/model/error.tsweb/packages/agenta-chat/src/model/parts.tsweb/packages/agenta-chat/src/state/expandState.tsweb/packages/agenta-chat/src/state/sessionEphemera.tsweb/packages/agenta-chat/src/state/sessionMessages.tsweb/packages/agenta-chat/src/transport/AgentChatTransport.tsweb/packages/agenta-chat/tests/unit/assets/attachmentRules.test.tsweb/packages/agenta-chat/tests/unit/assets/attachmentTransport.test.tsweb/packages/agenta-chat/tests/unit/assets/files.test.tsweb/packages/agenta-chat/tests/unit/assets/transcriptToMessages.test.tsweb/packages/agenta-chat/tests/unit/fixtures/approvalTurn.jsonweb/packages/agenta-chat/tests/unit/hooks/useAgentConversation.test.tsweb/packages/agenta-chat/tests/unit/hooks/useAttachmentUploads.test.tsweb/packages/agenta-chat/tests/unit/hooks/useComposerAttachments.test.tsweb/packages/agenta-chat/tests/unit/model/attachments.test.tsweb/packages/agenta-chat/tests/unit/state/sessionMessages.test.tsweb/packages/agenta-playground/package.jsonweb/packages/agenta-playground/src/agentChat.tsweb/packages/agenta-playground/src/state/execution/agentRequest.tsweb/packages/agenta-playground/src/state/execution/executionHeaders.tsweb/packages/agenta-playground/src/state/execution/webWorkerIntegration.ts
| /** Human summary of what is accepted, e.g. "Images, audio, and documents". */ | ||
| export const describeAccepted = (limits: AttachmentLimits): string => { | ||
| const nouns = limits.kinds.map((k) => KIND_NOUN[k]) | ||
| if (nouns.length === 0) return "No attachments" | ||
| const sentence = | ||
| nouns.length === 1 | ||
| ? nouns[0] | ||
| : `${nouns.slice(0, -1).join(", ")}, and ${nouns[nouns.length - 1]}` | ||
| return sentence.charAt(0).toUpperCase() + sentence.slice(1) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix the two-item join in describeAccepted.
When two kinds are enabled, the output is "Images, and audio". A comma before "and" is wrong for a two-item list. ComposerAttachments renders this string in the empty state, so users see it.
✏️ Proposed fix
const sentence =
nouns.length === 1
? nouns[0]
- : `${nouns.slice(0, -1).join(", ")}, and ${nouns[nouns.length - 1]}`
+ : nouns.length === 2
+ ? `${nouns[0]} and ${nouns[1]}`
+ : `${nouns.slice(0, -1).join(", ")}, and ${nouns[nouns.length - 1]}`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** Human summary of what is accepted, e.g. "Images, audio, and documents". */ | |
| export const describeAccepted = (limits: AttachmentLimits): string => { | |
| const nouns = limits.kinds.map((k) => KIND_NOUN[k]) | |
| if (nouns.length === 0) return "No attachments" | |
| const sentence = | |
| nouns.length === 1 | |
| ? nouns[0] | |
| : `${nouns.slice(0, -1).join(", ")}, and ${nouns[nouns.length - 1]}` | |
| return sentence.charAt(0).toUpperCase() + sentence.slice(1) | |
| } | |
| /** Human summary of what is accepted, e.g. "Images, audio, and documents". */ | |
| export const describeAccepted = (limits: AttachmentLimits): string => { | |
| const nouns = limits.kinds.map((k) => KIND_NOUN[k]) | |
| if (nouns.length === 0) return "No attachments" | |
| const sentence = | |
| nouns.length === 1 | |
| ? nouns[0] | |
| : nouns.length === 2 | |
| ? `${nouns[0]} and ${nouns[1]}` | |
| : `${nouns.slice(0, -1).join(", ")}, and ${nouns[nouns.length - 1]}` | |
| return sentence.charAt(0).toUpperCase() + sentence.slice(1) | |
| } |
| /** Build reference-carrying `file` parts for uploaded attachments (no inline bytes). */ | ||
| export const attachmentRefsToParts = (refs: AttachmentRef[], sessionId: string): FileUIPart[] => | ||
| refs.map((ref) => ({ | ||
| type: "file", | ||
| mediaType: ref.mediaType, | ||
| filename: ref.filename, | ||
| url: attachmentContentUrl(sessionId, ref.attachmentId), | ||
| providerMetadata: { | ||
| agenta: {attachmentId: ref.attachmentId, size: ref.size}, | ||
| }, | ||
| })) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find consumers that read the file part url vs. the attachment id.
rg -n -C4 --type=ts --type=tsx 'attachmentIdForPart|attachmentContentUrl' web/packages web/oss 2>/dev/null
rg -n -C3 --type=ts --type=tsx "part\.url|\.url\b" web/packages/agenta-chat/src 2>/dev/null | rg -n 'file|attachment' -iRepository: Agenta-AI/agenta
Length of output: 154
🏁 Script executed:
#!/bin/bash
set -u
echo "tracked files matching files.ts:"
git ls-files | rg 'web/packages/agenta-chat/src/assets/files\.ts$' || true
echo
echo "list assets files:"
git ls-files 'web/packages/agenta-chat/src/assets/*' || true
echo
echo "files.ts outline:"
ast-grep outline web/packages/agenta-chat/src/assets/files.ts 2>/dev/null || true
echo
echo "files.ts relevant content:"
cat -n web/packages/agenta-chat/src/assets/files.ts | sed -n '1,160p'
echo
echo "search attachment refs in repo:"
rg -n -C3 'attachmentIdForPart|attachmentContentUrl|attachmentRefsToParts|providerMetadata.*agenta|attachmentId' web/packages/agenta-chat src web 2>/dev/null | head -n 300Repository: Agenta-AI/agenta
Length of output: 35173
🏁 Script executed:
#!/bin/bash
set -u
echo "transcriptToMessages.ts relevant content:"
cat -n web/packages/agenta-chat/src/assets/transcriptToMessages.ts | sed -n '1,190p'
echo
echo "readers of attachmentIdForPart across tracked files:"
rg -n -C4 --glob '*.ts' --glob '*.tsx' 'attachmentIdForPart\(' . 2>/dev/null || true
echo
echo "readers of file part url in relevant package:"
rg -n -C3 --glob '*.ts' --glob '*.tsx' 'attachmentIdForPart|file\.url|\.url|FileUIPart|url:' web/packages/agenta-chat/src tests web 2>/dev/null | rg -n 'file|attachment|attachmentIdForPart|part\.url|\.url|providerMetadata' -C3 || trueRepository: Agenta-AI/agenta
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -u
echo "transcriptToMessages.ts top 170 lines:"
sed -n '1,170p' web/packages/agenta-chat/src/assets/transcriptToMessages.ts | cat -n
echo
echo "attachmentIdForPart call sites, tracked source only:"
git ls-files 'web/packages/agenta-chat/src/**/*.{ts,tsx}' 'web/packages/agenta-chat/tests/**/*.{ts,tsx}' | xargs rg -n -C3 'attachmentIdForPart\(' 2>/dev/null || true
echo
echo "file part url access in agenta-chat tracked src/tests:"
git ls-files 'web/packages/agenta-chat/src/**/*.{ts,tsx}' | xargs rg -n -C2 '\b(\.url|url:)' 2>/dev/null | rg -n -C2 'file|attachment|type.*file|FileUIPart|part\.url|\.url\b' || true
echo
echo "focused attachment content URL consumers, tracked source only:"
git ls-files 'web/packages/agenta-chat/src/**/*.{ts,tsx}' | xargs rg -n -C3 'attachmentContentUrl|providerMetadata' 2>/dev/null | head -n 240 || trueRepository: Agenta-AI/agenta
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -u
echo "transcriptToMessages.ts lines 1-170 byte size:"
wc -c web/packages/agenta-chat/src/assets/transcriptToMessages.ts
sed -n '1,170p' web/packages/agenta-chat/src/assets/transcriptToMessages.ts | cat -n
echo
echo "attachmentIdForPart call sites, tracked agenta-chat source/tests only:"
git ls-files 'web/packages/agenta-chat/src/**/*.{ts,tsx}' 'web/packages/agenta-chat/tests/**/*.{ts,tsx}' | xargs rg -n -C3 'attachmentIdForPart\(' 2>/dev/null || true
echo
echo "file part url access, tracked agenta-chat source/tests only:"
git ls-files 'web/packages/agenta-chat/src/**/*.{ts,tsx}' 'web/packages/agenta-chat/tests/**/*.{ts,tsx}' | xargs rg -n -C2 'part\.url|file\.url|\.url\b|type:\s*"file"' 2>/dev/null | rg -n -C2 'file|attachment|attachmentIdForPart|ProviderMetadata|part\.url|file\.url' || trueRepository: Agenta-AI/agenta
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -u
python3 - <<'PY'
from pathlib import Path
files = [
Path('web/packages/agenta-chat/src/assets/files.ts'),
Path('web/packages/agenta-chat/src/assets/transcriptToMessages.ts'),
Path('web/packages/agenta-chat/src/hooks/useComposerAttachments.ts'),
Path('web/packages/agenta-chat/tests/unit/assets/files.test.ts'),
Path('web/packages/agenta-chat/tests/unit/assets/transcriptToMessages.test.ts'),
]
for p in files:
print(f'\n--- {p} ({p.stat().st_size} bytes) ---')
text = p.read_text()
for needle in ['attachmentIdForPart', 'attachmentContentUrl', 'attachmentRefsToParts', '.url', 'part.url', 'file.url']:
lines = [(i+1, lines[i].strip()) for i, lines in enumerate(text.splitlines(keepends=True)) if needle in lines[i]]
if lines:
print(f'NEEDLE {needle!r}:')
for line, code in lines[:20]:
print(f'{line}: {code}')
PY
echo
echo "Direct targeted grep for attachmentIdForPart source usages:"
for f in $(git ls-files 'web/packages/agenta-chat/src/**/*.{ts,tsx}' 'web/packages/agenta-chat/tests/**/*.{ts,tsx}' 2>/dev/null); do
if rg -q 'attachmentIdForPart' "$f"; then
echo "--- $f ---"
rg -n -C3 'attachmentIdForPart' "$f"
fi
done
echo
echo "Direct targeted grep for file part url access in agenta-chat source/tests:"
for f in $(git ls-files 'web/packages/agenta-chat/src/**/*.{ts,tsx}' 'web/packages/agenta-chat/tests/**/*.{ts,tsx}' 2>/dev/null); do
if rg -q -E 'part\.(url|filename)|file\.(url|filename)|\btype:\s*"file"|attachmentIdForPart' "$f"; then
echo "--- $f ---"
rg -n -C2 -E 'part\.(url|filename)|file\.(url|filename)|\btype:\s*"file"|attachmentIdForPart' "$f" | head -n 80
fi
doneRepository: Agenta-AI/agenta
Length of output: 511
🏁 Script executed:
#!/bin/bash
set -u
python3 - <<'PY'
from pathlib import Path
files = [
Path('web/packages/agenta-chat/src/assets/files.ts'),
Path('web/packages/agenta-chat/src/assets/transcriptToMessages.ts'),
Path('web/packages/agenta-chat/src/hooks/useComposerAttachments.ts'),
Path('web/packages/agenta-chat/tests/unit/assets/files.test.ts'),
Path('web/packages/agenta-chat/tests/unit/assets/transcriptToMessages.test.ts'),
]
for p in files:
print(f'\n--- {p} ({p.stat().st_size} bytes) ---')
text = p.read_text()
for needle in ['attachmentIdForPart', 'attachmentContentUrl', 'attachmentRefsToParts', 'part.url', 'file.url', '.url']:
hits = '\n'.join(f'{i+1}: {line.strip()}' for i, line in enumerate(text.splitlines()) if needle in line)
if hits:
print(f'NEEDLE {needle!r}:')
print(hits)
else:
print(f'NEEDLE {needle!r}: no hits')
PY
echo
echo "Tracked agenta-chat source/tests lines mentioning attachmentIdForPart:"
for f in $(git ls-files 'web/packages/agenta-chat/src/**/*.{ts,tsx}' 'web/packages/agenta-chat/tests/**/*.{ts,tsx}' 2>/dev/null); do
if grep -q 'attachmentIdForPart' "$f"; then
echo "### $f"
grep -n -C 3 'attachmentIdForPart' "$f"
fi
done
echo
echo "Tracked agenta-chat source/tests lines mentioning reference file part url:"
for f in $(git ls-files 'web/packages/agenta-chat/src/**/*.{ts,tsx}' 'web/packages/agenta-chat/tests/**/*.{ts,tsx}' 2>/dev/null); do
if grep -qE 'type:\s*"file"|attachmentIdForPart|part\.url|file\.url' "$f"; then
echo "### $f"
grep -n -C 2 -E 'type:\s*"file"|attachmentIdForPart|part\.url|file\.url' "$f" | head -n 120
fi
doneRepository: Agenta-AI/agenta
Length of output: 2835
🏁 Script executed:
#!/bin/bash
set -u
echo "web/packages/agenta-chat/src/assets/transcriptToMessages.ts lines 120-152:"
sed -n '120,152p CARGO.toml web/packages/agenta-chat/src/assets/transcriptToMessages.ts' 2>/dev/null | sed 's#CARGO.toml#web/packages/agenta-chat/src/assets/transcriptToMessages.ts#' | cat -n
echo
echo "web/packages/agenta-chat/src/assets/transcriptToMessages.ts lines 260-300:"
sed -n '260,300p web/packages/agenta-chat/src/assets/transcriptToMessages.ts' | cat -n
echo
echo "Check transcript consumer lines for attachments/url in transcriptToMessages:"
sed -n '1,180p web/packages/agenta-chat/src/assets/transcriptToMessages.ts' | cat -n
echo
echo "Test expectation around transcript file part:"
sed -n '700,725p web/packages/agenta-chat/tests/unit/assets/transcriptToMessages.test.ts' | cat -n
echo
echo "Focused script: find transcript consumers mentioning fileUrl/FileUrl/url/content for attachments"
python3 - <<'PY'
from pathlib import Path
p = Path('web/packages/agenta-chat/src/assets/transcriptToMessages.ts')
text = p.read_text()
terms = 'fileUrl', 'FileUrl', 'content', 'attachments', 'attachmentId'
for i,line in enumerate(text.splitlines(), 1):
if any(t in line for t in terms):
print(f'{i}: {line}')
PY
echo
echo "Find direct consumers of transcriptsToMessages or transcriptToMessages in source/tests:"
for f in $(git ls-files 'web/packages/agenta-chat/src/**/*.{ts,tsx}' 'web/packages/agenta-chat/tests/**/*.{ts,tsx}' 2>/dev/null); do
if grep -q -E 'transcriptToMessages|transcriptsToMessages|messagesFrom' "$f"; then
echo "### $f"
grep -n -C 3 -E 'transcriptToMessages|transcriptsToMessages|messagesFrom' "$f" | head -n 120
fi
doneRepository: Agenta-AI/agenta
Length of output: 1519
Use attachment metadata when reconstructing persisted transcript file parts.
attachmentRefsToParts stores the attachmentId in providerMetadata.agenta, but transcript parsing still calls attachmentContentUrl(sessionId, attachmentId), which bakes the current API host into the persisted URL. When the API host changes, the stored part can point to the old domain. Rebuild the content URL from the current runtime API host and session id using the stored attachmentId instead of trusting the stored file.url.
| onClick={() => | ||
| onDenyAll(approvals.map((a) => a.approvalId)) | ||
| } | ||
| className="justify-start text-colorError hover:bg-[var(--ant-color-error-bg)]" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use a supported theme variable for the error background.
Line 351 uses the raw --ant-color-error-bg literal. The guidelines allow Ant Design semantic tokens, Tailwind color utilities, or var(--ag-color*) variables only. Replace it with a Tailwind token utility, for example hover:bg-colorErrorBg, and verify the hover state in light and dark themes.
As per coding guidelines: "Consume theme colors through Ant Design semantic tokens, Tailwind color utilities, or supported var(--ag-color*) variables; do not use raw hex colors or --ag-c-* literals."
Source: Coding guidelines
| useEffect(() => { | ||
| const el = audioRef.current | ||
| if (!el) return | ||
| // A new source voids any probe from the previous one. Without this reset, a src swap mid-probe | ||
| // leaves probingRef stuck true, which gates off onTimeUpdate below and freezes the timer. | ||
| probingRef.current = false | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reset duration and current when src changes.
The effect resets probingRef but keeps the duration and current state of the previous source. Until loadedmetadata fires for the new source, the UI shows the old elapsed time and the old total, and the progress bar renders at the old fill.
🐛 Proposed fix
probingRef.current = false
+ setCurrent(0)
+ setDuration(0)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect(() => { | |
| const el = audioRef.current | |
| if (!el) return | |
| // A new source voids any probe from the previous one. Without this reset, a src swap mid-probe | |
| // leaves probingRef stuck true, which gates off onTimeUpdate below and freezes the timer. | |
| probingRef.current = false | |
| useEffect(() => { | |
| const el = audioRef.current | |
| if (!el) return | |
| // A new source voids any probe from the previous one. Without this reset, a src swap mid-probe | |
| // leaves probingRef stuck true, which gates off onTimeUpdate below and freezes the timer. | |
| probingRef.current = false | |
| setCurrent(0) | |
| setDuration(0) |
| <div | ||
| role={onClick ? "button" : undefined} | ||
| onClick={onClick} | ||
| className={`flex ${TILE} items-center gap-2 rounded-lg border border-solid border-colorBorderSecondary bg-colorFillQuaternary px-2 ${onClick ? "cursor-pointer hover:border-colorBorder" : ""} ${className ?? ""}`} |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
role="button" elements are not keyboard operable.
The Chip wrapper at Line 146 and the image tile at Line 334 take role="button" and an onClick, but neither sets tabIndex nor handles keyboard activation. A keyboard user cannot open an attachment in the viewer. The Remove control is a real <button>, so only the view action is unreachable.
🐛 Proposed fix
<div
role={onClick ? "button" : undefined}
+ tabIndex={onClick ? 0 : undefined}
onClick={onClick}
+ onKeyDown={
+ onClick
+ ? (e) => {
+ if (e.key === "Enter" || e.key === " ") {
+ e.preventDefault()
+ onClick()
+ }
+ }
+ : undefined
+ }
className={`flex ${TILE} items-center gap-2 rounded-lg border border-solid border-colorBorderSecondary bg-colorFillQuaternary px-2 ${onClick ? "cursor-pointer hover:border-colorBorder" : ""} ${className ?? ""}`}
>Apply the same tabIndex and onKeyDown pair to the image tile at Line 333, and add a visible focus ring to both so the focus position is clear in light and dark themes.
Also applies to: 333-339
| const abort = useCallback((uid: string) => { | ||
| queued.current.delete(uid) | ||
| retryAt.current.delete(uid) | ||
| retryable.current.delete(uid) | ||
| clearTimeout(retryTimers.current.get(uid)) | ||
| retryTimers.current.delete(uid) | ||
| const controller = controllers.current.get(uid) | ||
| controllers.current.delete(uid) | ||
| controller?.abort() | ||
| }, []) | ||
| const enqueue = useCallback((uids: string[]) => { | ||
| uids.forEach((uid) => queued.current.add(uid)) | ||
| }, []) | ||
| useEffect(() => { | ||
| // A remounted tray resumes in-flight entries with the same uid/idempotency key. | ||
| for (const file of files) { | ||
| if ( | ||
| file.status === "uploading" && | ||
| file.originFileObj && | ||
| !controllers.current.has(file.uid) | ||
| ) { | ||
| queued.current.add(file.uid) | ||
| } | ||
| } | ||
| for (const uid of queued.current) { | ||
| if (!files.some((file) => file.uid === uid)) continue | ||
| queued.current.delete(uid) | ||
| run(uid) | ||
| } | ||
| }, [files, run]) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
abort does not stop an upload; the resume effect restarts it.
abort deletes the controller and cancels the request, but it never changes the row's status. The row stays status: "uploading" with originFileObj set. On the next files change the effect at Line 131 finds a row that is uploading and has no controller, re-queues it, and run starts the upload again. Only removeUploadFile stops an upload for good, because it also removes the row.
abort is part of the exported AttachmentUploads contract and reaches hosts through useComposerAttachments().uploads and ChatComposer. A host that cancels one upload without removing the chip will see it resume.
Mark aborted rows so the resume rule skips them.
🐛 Proposed fix
const abort = useCallback((uid: string) => {
queued.current.delete(uid)
retryAt.current.delete(uid)
retryable.current.delete(uid)
clearTimeout(retryTimers.current.get(uid))
retryTimers.current.delete(uid)
const controller = controllers.current.get(uid)
controllers.current.delete(uid)
controller?.abort()
+ aborted.current.add(uid)
}, []) useEffect(() => {
// A remounted tray resumes in-flight entries with the same uid/idempotency key.
for (const file of files) {
if (
file.status === "uploading" &&
file.originFileObj &&
+ !aborted.current.has(file.uid) &&
!controllers.current.has(file.uid)
) {
queued.current.add(file.uid)
}
}Add const aborted = useRef(new Set<string>()) beside the other refs, and aborted.current.delete(uid) at the start of run so a retry clears the flag.
Run the following script to find hosts that call abort without removing the row:
#!/bin/bash
# Locate uploads.abort call sites outside removeUploadFile.
rg -nP --type=ts --type=tsx -C4 '\buploads\.abort\s*\(|\babort\s*\(\s*uid' web/packages -g '!**/node_modules/**'
rg -nP --type=ts --type=tsx -C4 '\bremoveUploadFile\s*\(' web/packages -g '!**/node_modules/**'| // Restored from the per-session store on remount (route re-entry, tab close/reopen) — | ||
| // pending attachments survive alongside the composer draft. Rejections stay transient. | ||
| const [files, setFiles] = useState<PendingAttachment[]>(() => | ||
| sessionId ? (attachmentsBySession.get(sessionId) ?? []) : [], | ||
| const [files, setFiles] = useState<StagedFile[]>( | ||
| () => (attachmentsBySession.get(sessionId) as StagedFile[] | undefined) ?? [], | ||
| ) | ||
| // `sessionId` is a prop. Today's only caller mounts one instance per session, but a caller | ||
| // that swapped it in place would carry the old session's staged files into the new one, and | ||
| // the mirror effect below would then write them under the new key. Re-seed during render so | ||
| // that effect never sees a mismatched pair. | ||
| const [seededFor, setSeededFor] = useState(sessionId) | ||
| if (sessionId !== seededFor) { | ||
| setSeededFor(sessionId) | ||
| setFiles(sessionId ? (attachmentsBySession.get(sessionId) ?? []) : []) | ||
| } | ||
| useEffect(() => { | ||
| if (!sessionId) return | ||
| if (files.length > 0) attachmentsBySession.set(sessionId, files) | ||
| else attachmentsBySession.delete(sessionId) | ||
| }, [files, sessionId]) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
A sessionId change on a mounted hook moves staged files into the new session.
The useState initializer at Line 62 runs only on mount. If sessionId changes while the hook stays mounted, files keeps the previous session's rows, and the effect at Line 65 writes those rows under the new sessionId. The previous session's map entry is also orphaned. Because attachmentUploader closes over sessionId, a retry then uploads a file staged for session A into session B.
Nothing in the hook enforces the remount that the comment assumes. Either resynchronize on sessionId change, or require hosts to key the component by session.
🐛 Proposed fix — resynchronize on session change
const [files, setFiles] = useState<StagedFile[]>(
() => (attachmentsBySession.get(sessionId) as StagedFile[] | undefined) ?? [],
)
+ const loadedSession = useRef(sessionId)
+ if (loadedSession.current !== sessionId) {
+ loadedSession.current = sessionId
+ setFiles((attachmentsBySession.get(sessionId) as StagedFile[] | undefined) ?? [])
+ }
useEffect(() => {
if (files.length > 0) attachmentsBySession.set(sessionId, files)
else attachmentsBySession.delete(sessionId)
}, [files, sessionId])Run the following script to check whether hosts remount the composer per session:
#!/bin/bash
# Find useComposerAttachments call sites and check for a session-based React key on the owning component.
rg -nP --type=ts --type=tsx -C10 'useComposerAttachments\s*\(' web -g '!**/node_modules/**'
rg -nP --type=tsx -C3 '<ChatComposer\b' web -g '!**/node_modules/**'| const uploadExtraFiles = async (extraFiles: File[]): Promise<StagedFile[] | null> => { | ||
| const staged = extraFiles.map((file) => toUploadFile(file, uploadsEnabled)) | ||
| const results = await Promise.allSettled( | ||
| staged.map(async (entry) => { | ||
| const response = await attachmentUploader(entry.originFileObj as File, { | ||
| uid: entry.uid, | ||
| onProgress: () => undefined, | ||
| signal: new AbortController().signal, | ||
| }) | ||
| return {...entry, status: "done" as const, percent: 100, response} | ||
| }), | ||
| ) | ||
| if (results.every((result) => result.status === "fulfilled")) { | ||
| return results.map((result) => (result as PromiseFulfilledResult<StagedFile>).value) | ||
| } | ||
| const settledEntries = results.map((result, index) => | ||
| result.status === "fulfilled" | ||
| ? result.value | ||
| : { | ||
| ...staged[index], | ||
| status: "error" as const, | ||
| error: | ||
| result.reason instanceof Error ? result.reason.message : "Upload failed", | ||
| }, | ||
| ) | ||
| setFiles((prev) => [...prev, ...settledEntries]) | ||
| setAttachmentsOpen(true) | ||
| return null | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
uploadExtraFiles creates an AbortController it immediately discards.
Line 148 passes new AbortController().signal, and the controller is never retained. These uploads cannot be cancelled. They keep running after the composer unmounts or the user switches session, because useAttachmentUploads holds no controller for these uids either — run is bypassed.
Hold one controller per entry and abort them on unmount.
🐛 Proposed fix
+ // Controllers for send-time uploads that never entered the tray.
+ const extraControllers = useRef(new Map<string, AbortController>())
+ useEffect(
+ () => () => {
+ extraControllers.current.forEach((controller) => controller.abort())
+ extraControllers.current.clear()
+ },
+ [],
+ )
+
const uploadExtraFiles = async (extraFiles: File[]): Promise<StagedFile[] | null> => {
const staged = extraFiles.map((file) => toUploadFile(file, uploadsEnabled))
const results = await Promise.allSettled(
staged.map(async (entry) => {
- const response = await attachmentUploader(entry.originFileObj as File, {
- uid: entry.uid,
- onProgress: () => undefined,
- signal: new AbortController().signal,
- })
- return {...entry, status: "done" as const, percent: 100, response}
+ const controller = new AbortController()
+ extraControllers.current.set(entry.uid, controller)
+ try {
+ const response = await attachmentUploader(entry.originFileObj as File, {
+ uid: entry.uid,
+ onProgress: () => undefined,
+ signal: controller.signal,
+ })
+ return {...entry, status: "done" as const, percent: 100, response}
+ } finally {
+ extraControllers.current.delete(entry.uid)
+ }
}),
)Separately, this path uploads even when uploadsEnabled is false, while toUploadFile marks the entries done for that same flag. Confirm that the inline-only rollout state is meant to allow send-time uploads.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const uploadExtraFiles = async (extraFiles: File[]): Promise<StagedFile[] | null> => { | |
| const staged = extraFiles.map((file) => toUploadFile(file, uploadsEnabled)) | |
| const results = await Promise.allSettled( | |
| staged.map(async (entry) => { | |
| const response = await attachmentUploader(entry.originFileObj as File, { | |
| uid: entry.uid, | |
| onProgress: () => undefined, | |
| signal: new AbortController().signal, | |
| }) | |
| return {...entry, status: "done" as const, percent: 100, response} | |
| }), | |
| ) | |
| if (results.every((result) => result.status === "fulfilled")) { | |
| return results.map((result) => (result as PromiseFulfilledResult<StagedFile>).value) | |
| } | |
| const settledEntries = results.map((result, index) => | |
| result.status === "fulfilled" | |
| ? result.value | |
| : { | |
| ...staged[index], | |
| status: "error" as const, | |
| error: | |
| result.reason instanceof Error ? result.reason.message : "Upload failed", | |
| }, | |
| ) | |
| setFiles((prev) => [...prev, ...settledEntries]) | |
| setAttachmentsOpen(true) | |
| return null | |
| } | |
| // Controllers for send-time uploads that never entered the tray. | |
| const extraControllers = useRef(new Map<string, AbortController>()) | |
| useEffect( | |
| () => () => { | |
| extraControllers.current.forEach((controller) => controller.abort()) | |
| extraControllers.current.clear() | |
| }, | |
| [], | |
| ) | |
| const uploadExtraFiles = async (extraFiles: File[]): Promise<StagedFile[] | null> => { | |
| const staged = extraFiles.map((file) => toUploadFile(file, uploadsEnabled)) | |
| const results = await Promise.allSettled( | |
| staged.map(async (entry) => { | |
| const controller = new AbortController() | |
| extraControllers.current.set(entry.uid, controller) | |
| try { | |
| const response = await attachmentUploader(entry.originFileObj as File, { | |
| uid: entry.uid, | |
| onProgress: () => undefined, | |
| signal: controller.signal, | |
| }) | |
| return {...entry, status: "done" as const, percent: 100, response} | |
| } finally { | |
| extraControllers.current.delete(entry.uid) | |
| } | |
| }), | |
| ) | |
| if (results.every((result) => result.status === "fulfilled")) { | |
| return results.map((result) => (result as PromiseFulfilledResult<StagedFile>).value) | |
| } | |
| const settledEntries = results.map((result, index) => | |
| result.status === "fulfilled" | |
| ? result.value | |
| : { | |
| ...staged[index], | |
| status: "error" as const, | |
| error: | |
| result.reason instanceof Error ? result.reason.message : "Upload failed", | |
| }, | |
| ) | |
| setFiles((prev) => [...prev, ...settledEntries]) | |
| setAttachmentsOpen(true) | |
| return null | |
| } |
| * are globally unique, so this store has no scope dimension. | ||
| * v2: caches written by the pre-fix mapper hold duplicated approval parts; the key bump forces | ||
| * one re-sync from records (the watermark otherwise keeps the stale copy authoritative). */ | ||
| export const sessionMessagesAtom = atomWithStorage<Record<string, UIMessage[]>>( | ||
| "agenta:agent-chat:messages", | ||
| "agenta:agent-chat:messages:v2", |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Expect: migration or cleanup for pre-v2 keys and deletion coverage.
rg -n -C 8 \
'agenta:agent-chat:(messages|record-counts)(:v2)?|dropSessionMessagesAtom|removeItem|migrat' \
web/packagesRepository: Agenta-AI/agenta
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate files =="
fd -a 'sessionMessages|useAgentConversation' web/packages/agenta-chat/src | sed 's#^\./##'
echo
echo "== sessionMessages outline =="
ast-grep outline web/packages/agenta-chat/src/state/sessionMessages.ts --view compact || true
echo
echo "== sessionMessages relevant lines =="
nl -ba web/packages/agenta-chat/src/state/sessionMessages.ts | sed -n '1,140p'
echo
echo "== useAgentConversation relevant lines =="
nl -ba web/packages/agenta-chat/src/hooks/useAgentConversation.ts | sed -n '120,330p'
echo
echo "== focused key references in chat package =="
rg -n -C 5 'agenta:agent-chat:messages|record-counts|dropSessionMessagesAtom|removeItem|localStorage|setItem|getItem|re.*sync|sync' web/packages/agenta-chat/srcRepository: Agenta-AI/agenta
Length of output: 588
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== sessionMessages.ts =="
awk '{printf "%6d: %s\n", NR, $0}' web/packages/agenta-chat/src/state/sessionMessages.ts | sed -n '1,180p'
echo
echo "== useAgentConversation.ts relevant section =="
awk '{printf "%6d: %s\n", NR, $0}' web/packages/agenta-chat/src/hooks/useAgentConversation.ts | sed -n '120,340p'
echo
echo "== focused key references in chat package =="
rg -n -C 5 'agenta:agent-chat:messages|record-counts|dropSessionMessagesAtom|removeItem|localStorage|setItem|getItem|re.*sync|sync' web/packages/agenta-chat/src
echo
echo "== all tracked references to pre-v2 keys =="
rg -n 'agenta:agent-chat:(messages|record-counts)(?!:v2)|dropSessionMessagesAtom' web
echo
echo "== diff stat/name-only for context =="
git diff --stat || true
git diff --name-only || trueRepository: Agenta-AI/agenta
Length of output: 50372
Clean up the pre-v2 transcript keys.
dropSessionMessagesAtom only deletes entries in the :v2 stores, and the hydration/revalidation path reads sessionMessagesAtom, so existing agenta:agent-chat:messages / agenta:agent-chat:record-counts data can remain in localStorage. Add a one-time migration or delete those legacy keys on successful v2 re-sync.
| const delay = Math.min(SMOOTH_BASE_MS, SMOOTH_CHUNK_BUDGET_MS / pieces.length) | ||
| for (const piece of pieces) { | ||
| controller.enqueue({...(value as object), delta: piece} as AnyChunk) | ||
| if (delay >= 1) await sleep(delay) | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Keep pacing and backpressure for large deltas.
For more than 1,200 pieces, delay is less than one. Line 248 then emits every piece in one pull() call. A large server delta can create thousands of chunks without yielding or honoring controller.desiredSize.
Keep pending pieces across pulls, or schedule bounded batches. Do not disable yielding when the chunk exceeds the time budget.
…stops churning previews Four defects in @agenta/chat's composer attachments, all in the same subsystem. `uploads.abort(uid)` only paused an upload. It released the controller but left the row reading "uploading" — precisely the shape the remount-resume rule restarts — so the next `files` identity relaunched the request, which mid-upload is many times a second. It now marks the uid cancelled for this mount AND settles the row as a retryable error, while an unmount-abort (a different code path) still resumes. `ComposerAttachments` rebuilt every preview object URL whenever the array identity changed, so an upload's progress ticks revoked and re-minted every URL continuously: thumbnails flickered, audio playback restarted, and a decode against a just-revoked URL latched the broken-image fallback permanently (nothing ever cleared it). URLs are now keyed by uid, minted once, and released when the row or its blob goes away. `uploadExtraFiles` built an AbortController and threw it away, so those uploads could never be cancelled. They are held and aborted on unmount now. A `sessionId` change on a mounted hook wrote session A's staged rows under session B — the state initializer runs only on mount, and nothing makes hosts remount per session (the chat workspace keeps tab panes mounted). The rows now move back under their own session before the incoming one is adopted. Also makes the tile view affordance a real button instead of a `role="button"` div with no tabIndex and no key handling — as a sibling of the Remove button, not a wrapper around it.
…place `useAgentConversation` carried two near-identical `adopt` closures plus a third inline copy in the hydration path, and all three reconciled by MESSAGE COUNT alone — the comment said so outright. `transcriptToMessages` folds a paused turn into its resume and only closes a message on `done`, so a turn that completes IN PLACE (an approval resolving into the same assistant message) leaves the count unchanged and the finished server transcript was skipped. `transcript.recordCount` was persisted on every path and never compared. The three copies collapse into one `adoptServerTranscript` that runs the shared `shouldAdoptServerTranscript` rule: the record watermark is the trigger, the message count only a floor. The copies had already drifted — one cleared the history-unavailable notice and the other didn't; adopting real history settles that question, so it clears everywhere. The watermark is seeded from the persisted store, cleared when a live turn supersedes the transcript, and filed with the persist. Two smaller data-integrity fixes alongside: `filePartContentUrl` rebuilds an attachment's content URL from the current runtime API host plus the stored attachment id, rather than trusting a `url` that was baked when the part was persisted — a host change otherwise leaves stored parts pointing at the old domain. Parts with no attachment id (inline data URLs, anything predating the metadata) keep their stored URL. The pre-`:v2` localStorage stores are dropped once at load. Nothing reads them, but `dropSessionMessagesAtom` only ever deleted the `:v2` keys, so they sat there taking quota from the stores that are used.
…age fixes `@agenta/chat` is consumed by the mobile app, which does not load antd's CSS variables, so the three raw `var(--ant-color-error-bg)` literals resolved to nothing there — on top of violating the repo rule against raw `--ant-*` literals. They become the `colorErrorBg` Tailwind token the rest of the package already uses. The smooth-stream pacing collapsed on large deltas: above ~1,200 pieces the per-piece delay drops below a millisecond, the sleep was skipped entirely, and the whole delta was flushed from a single `pull()` — thousands of enqueues with no yield and no further look at the consumer. Pieces now carry across `pull()` calls and a pull stops once the consumer is full; the cadence for normal-sized deltas is unchanged. The new test pins what actually matters: the text still reassembles byte-identically, in order. `describeAccepted` wrote "Images, and audio" — a two-item list takes a bare "and". It renders in the composer's empty state. `AudioPlayer` left the previous source's duration and elapsed time on screen until `loadedmetadata` fired for the new one; both reset with the src now.
617face to
5071cfc
Compare
9a75085 to
954118f
Compare
1 participant
@agenta/chatis the chat runtime as a package: the message engine, the composer, and theapproval card. It is the dependency both the OSS chat slice (next lane) and
/mbuild on.antd-free by construction — the mobile app cannot take an antd dependency.
Not run in a browser — static gates only (
pnpm lint-fix24/24,tsc --noEmitcleanfor
@agenta/shared,ui,entities,entity-ui,settings-ui,oss,ee,mobile).Stacked on
pkg/session-surfaces; review only this lane's diff.