VOOZH about

URL: https://phabricator.services.mozilla.com/D304832

⇱ ⚙ D304832 Bug 2040449 - Handle restored chats with incomplete or abandoned browser actions r=omarg,yjamora

Paths

Bug 2040449 - Handle restored chats with incomplete or abandoned browser actions r=omarg,yjamora
ClosedPublic

Authored by ngrato on Thu, Jun 4, 4:33 PM.
Referenced Files
F72890526: D304832.1781753056.diff
Wed, Jun 17, 3:24 AM
Unknown Object (File)
Tue, Jun 16, 10:15 PM
Unknown Object (File)
Tue, Jun 16, 10:52 AM
Unknown Object (File)
Mon, Jun 15, 4:18 AM
Unknown Object (File)
Sun, Jun 14, 8:39 PM
Unknown Object (File)
Sun, Jun 14, 6:20 PM
Unknown Object (File)
Sun, Jun 14, 11:45 AM
Unknown Object (File)
Sun, Jun 14, 9:01 AM

Diff Detail

Event Timeline

ngrato created this revision.
phab-bot changed the visibility from "Custom Policy" to "Public (No Login Required)".Thu, Jun 4, 4:35 PM
phab-bot changed the edit policy from "Custom Policy" to "Restricted Project (Project)".
phab-bot removed a project: secure-revision.
ngrato updated this revision to Diff 1292282.
ngrato updated this revision to Diff 1292285.
ngrato requested review of this revision.Thu, Jun 4, 5:08 PM
ngrato retitled this revision from WIP: Bug 2040449 - Handle restored chats with incomplete or abandoned browser actionsr=omarg,yjamora to Bug 2040449 - Handle restored chats with incomplete or abandoned browser actionsr=omarg,yjamora.
ngrato retitled this revision from Bug 2040449 - Handle restored chats with incomplete or abandoned browser actionsr=omarg,yjamora to Bug 2040449 - Handle restored chats with incomplete or abandoned browser actions r=omarg,yjamora.
omarg requested changes to this revision.Thu, Jun 4, 8:38 PM
omarg added inline comments.
browser/components/aiwindow/ui/components/ai-chat-content/ai-chat-content.mjs
1200

nit: If these properties are always going to come from the why not just:

this.#renderToolUI(msg);

and then:

#renderToolUI(message) {
 // Since these are used in a switch you'll just need to destructure these at the top
 // or destructure in the function signature. I'd think we're going to keep adding more
 // tool uis that are going to keep wanting more and more fields to use
 const { toolUIData, messageId, isRestored = false } = message;
}
browser/components/aiwindow/ui/components/ai-window/ai-window.mjs
2102–2113 ↗(On Diff #1292286)

This feels like the wrong place for this.

Maybe should be here instead?
https://searchfox.org/firefox-main/source/browser/components/aiwindow/ui/modules/ChatStore.sys.mjs#890

browser/components/aiwindow/ui/modules/ChatConversation.sys.mjs
1124–1137 ↗(On Diff #1292286)

Feels kind of weird to mutate the since it's an argument here.

Maybe it's better to mutate it before passing it in here: https://searchfox.org/firefox-main/source/browser/components/aiwindow/models/Chat.sys.mjs#139

This revision now requires changes to proceed.Thu, Jun 4, 8:38 PM
ngrato requested review of this revision.Mon, Jun 8, 9:29 PM
ngrato updated this revision to Diff 1294419.
ngrato marked 3 inline comments as done.
ngrato updated this revision to Diff 1296855.
Comment Actions

Summary

Intent

The goal of these changes is to handle a scenario where a user has an in-progress browser action (specifically a "website-confirmation" dialog, such as confirming which tabs to close) that was never completed or was abandoned, and then the chat session is restored from the database. Without this fix, restoring such a conversation would re-display the original confirmation UI with stale data (e.g., tab references that may no longer be valid). The fix ensures that these incomplete confirmations are detected upon restoration and presented to the user as a retry prompt instead.

Solution

The solution involves several coordinated changes across the chat store and the UI rendering layer:

  1. Database restoration flagging (ChatStore.sys.mjs): When messages are loaded from the database, any message whose has a of is marked with an flag. This signals that the confirmation was persisted but never completed. A new import of from the module is added to support this check.
  1. Propagating the flag through the UI (ai-chat-content.mjs): The property is extracted from incoming message event details and passed through to the conversation state, ensuring the flag is available during rendering.
  1. Refactoring and related methods: The method signature is changed to accept the full message object instead of separate and parameters. This same refactoring applies to , , and . This allows these methods to access the flag and other message properties as needed.
  1. Rendering a retry UI for restored confirmations: In , when the message has the flag set, the method renders a retry component (with a localized message and retry button) instead of the original tab-confirmation UI. The retry button uses the from the tool data so the user can re-initiate the action with fresh context.
  1. Tests (test_ChatStore.js): Two new tests verify the behavior: one confirms that messages with toolUIData receive the flag when loaded from the database, and another confirms that messages with other UI types (e.g., ) do not receive the flag.
Please use / reactions on inline comments to provide feedback. This will have a significant impact on the quality of future reviews.
browser/components/aiwindow/ui/components/ai-chat-content/ai-chat-content.mjs
1060

This retry template is a near-exact duplicate of . The only difference is the addition of on the . Reuse here instead of duplicating the markup. If the styling is intentional for the restored case, add that as a parameter to rather than copying the whole block.

1073

The attribute is present here but absent from the identical template in . This means the retry button renders with different visual styling depending on whether it's a restored website-confirmation or a normal retry. If this is intentional, it should be documented; if not, make both consistent.

browser/components/aiwindow/ui/test/xpcshell/test_ChatStore.js
1776

The key passed here is silently ignored because destructures only . The resulting will be , not . Remove the misleading argument, or extend to accept it.

ngrato marked 3 inline comments as done.
This revision is now accepted and ready to land.Fri, Jun 12, 2:39 AM

Revision Contents

PathSize
browser/
components/
aiwindow/
ui/
components/
ai-chat-content/
46 lines
modules/
4 lines
test/
xpcshell/
95 lines
CommitTreeParentsAuthorSummaryDate
e0a321b9f3dad5ef729a42d3nickgrato
Bug 2040449 - Handle restored chats with incomplete or abandoned browser… (Show More…)

Diff 1298974

browser/components/aiwindow/ui/components/ai-chat-content/ai-chat-content.mjs

Loading...

browser/components/aiwindow/ui/modules/ChatStore.sys.mjs

Loading...

browser/components/aiwindow/ui/test/xpcshell/test_ChatStore.js

Loading...