Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bb7cbba654
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| Status: proposed | ||
| Translation: current | ||
| Translation: pending |
There was a problem hiding this comment.
Translate the newly added “Pending failure hierarchy” section into the adjacent Chinese note and mark both copies Translation: current. This commit changes only the English document to pending, while the Chinese counterpart still claims current, leaving the two decision records inconsistent; agent-authored notes must land both language versions together and may not use pending.
AGENTS.md reference: .agents/notes/AGENTS.md:L91-L96
Useful? React with 👍 / 👎.
bb7cbba to
e5b66a1
Compare
e5b66a1 to
ba1669a
Compare
ba1669a to
fe742d7
Compare
fe742d7 to
53f216b
Compare
53f216b to
ae84758
Compare
ae84758 to
eb2dc7e
Compare
The pending row printed the same failure three times — a message-level
"Not sent · Attachment upload failed", the record's own error string, and
each failed attachment's reason — so a partly failed message read as a
diagnostic dump rather than a message.
Rendering now assigns one owner per fact. The message level keeps one
short neutral status ("Not sent"); the reason belongs to the attachment
that failed, and the record-level reason renders only when no attachment
carries one, so a journal-level failure still explains itself without
ever duplicating a reason. A reason with no card to live on arrives
through PendingFailureNotice in the same frame instead of loose red text
under the bubble.
FAILED_FRAME_CLASS is the whole red budget: only the failed card takes
it, while the icon tile, filename and message status stay neutral. The
image card's failure uses a neutral scrim rather than a red wash.
Uploading / ready / failed share one card skeleton, and the progress bar
is a flush strip that always reserves its row, so a card keeps one height
across all three states (image 198px, file 66px, measured). The trailing
state glyphs cross-fade on opacity/scale/blur in CSS rather than pulling
framer-motion into the conversation's module graph.
Rendering only: journal stages, Effect lifetimes, retry/cancel semantics,
ordering and persistence are untouched.
Model: claude-opus-5
Model: gpt-6
eb2dc7e to
87b1e75
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 87b1e75f6a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| <p | ||
| className={cn( | ||
| 'truncate text-[11px]', | ||
| failed ? 'text-destructive' : 'text-muted-foreground' | ||
| )} |
There was a problem hiding this comment.
When an attachment fails on the narrow/mobile layout, this single-line truncate hides most of the reason (including the story's “Network error while preparing the file”), while reasonOnACard suppresses the full record-level notice. The title attribute does not provide a usable disclosure on touch devices, so affected users see only “Not sent” and an incomplete cause before retrying; wrap the failure text or provide a touch/keyboard-accessible way to reveal it. The file-card status at lines 240–246 has the same problem.
Useful? React with 👍 / 👎.
This branch has not been deployed
1 participant
Stacked on #712 (
feat/attachment-drafts). Rendering only.The problem
The pending row printed the same failure three times — a message-level "Not sent · Attachment upload failed", the record's own
errorstring, and each failed attachment's reason. Buttons sat in a permanently-rendered emptydiv, and the failed card was not visually distinct from a finished one, so a partly failed message read as a debug panel rather than a message.One owner per fact
Not sent · Attachment upload failed, intext-destructiveNot sent, intext-muted-foregrounderror<p>under the bubblePendingFailureNotice, and only when no attachment carries a reasonThe record-level reason is not simply dropped: a journal-level failure can occur with every attachment ready, so it still renders — just never alongside a per-attachment reason. One fault is stated once.
Red budget
FAILED_FRAME_CLASS(border-destructive/30 bg-destructive/[0.04]) is the entire red budget, and only a failed card takes it. The icon tile, filename and message status stay neutral; the image card's failure uses a neutralbg-background/55scrim rather than a red wash. A failed message now has three red things (faint frame, oneAlertCircle, one reason line) instead of seven.Layout
Uploading / ready / failed share one card skeleton — icon slot, name plus one status line, trailing state glyph. The progress bar is a full-width strip flush on the card's bottom edge that always reserves its row, so a card keeps one height across all three states. Measured in the browser: image card 198px and file card 66px in uploading, failed and prepared alike.
Absolute positioning was tried first and was wrong — at
bottom-2the bar overlapped the 40px content row by 2px and left an unrelated 8px gap beneath it.The three trailing glyphs stay mounted and cross-fade on opacity/scale/blur (0→1, 0.25→1, 4px→0, 300ms), with the transition naming its exact properties. CSS rather than framer-motion: nothing else under
chat/orai-gui/pulls that dependency into the conversation's module graph. File cards sharegetSessionFileIconwith the deliveredSessionFileCard, so a draft and its sent form read as one object.Scope
Rendering layer only. Journal stages, Effect lifetimes, retry/cancel semantics, message ordering and persistence are untouched.
PendingMessageRowis exported as a pure component so Storybook and tests drive the states without a workspace runtime.Verification
tsgo), oxlint,lint:i18n, prettier andpnpm docs check(0 errors) all clean.opacity/scale/filterandtransition-propertywere read from the live DOM rather than assumed (Tailwind v4 emits the standalonescaleproperty, so a naivetransformcheck looks broken).Notes for the reviewer
data-attachment-progresshook, not a style selector:Progressmerges to the sameh-1 w-fulland is indistinguishable by styling alone.sessions.pendingMessageUploadFailedchanged value inen/zh_CN(edited textually — these are flat-key files with duplicates that a JSON round-trip would reorder).Translation:flipped topending; the Chinese version of the new section is outstanding.Pre-existing on the base branch, not touched here
chat-composer.tsx:867renders a baret('sessions.attachmentDraft')string where sibling branches render a positioned progress<div>.components/chat/AGENTS.mdstill states "Submit blocks whilehasBlockingImagesorhasBlockingFiles", but the base branch hard-disabled both (= false).🤖 Generated with Claude Code