commit 5b981937407092d5c5671ea41980709fea5fcc6d
parent 7439d6aacf5dde98a8750272a790d6919486847d
Author: Christopher DiPersio <cdipersio@mozilla.com>
Date: Mon, 15 Dec 2025 23:03:48 +0000
Bug 2003674 - Delete insights in InsightManager r=cgopal,ai-models-reviewers,ai-ondevice-reviewers,atossou
Differential Revision: https://phabricator.services.mozilla.com/D275801
Diffstat:
4 files changed, 219 insertions(+), 17 deletions(-)
diff --git a/browser/base/content/test/static/browser_all_files_referenced.js b/browser/base/content/test/static/browser_all_files_referenced.js
@@ -360,10 +360,6 @@ var allowlist = [
{
file: "moz-src:///browser/components/aiwindow/models/Tools.sys.mjs",
},
- // Bug 2003671 - Fetch Insights - getRelevantInsights
- {
- file: "moz-src:///browser/components/aiwindow/models/InsightsManager.sys.mjs",
- },
// Bug 2004888 - [FirstRun] Create Firstrun.html opening firstrun welcome screen
{
file: "chrome://browser/content/aiwindow/firstrun.html",
diff --git a/browser/components/aiwindow/models/InsightsManager.sys.mjs b/browser/components/aiwindow/models/InsightsManager.sys.mjs
@@ -205,6 +205,9 @@ export class InsightsManager {
* Retrieves all stored insights.
* This is a quick-access wrapper around InsightStore.getInsights() with no additional processing.
*
+ * @param {object} [opts={}]
+ * @param {boolean} [opts.includeSoftDeleted=false]
+ * Whether to include soft-deleted insights.
* @returns {Promise<Array<Map<{
* insight_summary: string,
* category: string,
@@ -212,8 +215,8 @@ export class InsightsManager {
* score: number,
* }>>>} List of insights
*/
- static async getAllInsights() {
- return await InsightStore.getInsights();
+ static async getAllInsights(opts = { includeSoftDeleted: false }) {
+ return await InsightStore.getInsights(opts);
}
/**
@@ -288,6 +291,32 @@ export class InsightsManager {
}
/**
+ * Soft deletes an insight by its ID.
+ * Soft deletion sets the insight's `is_deleted` flag to true. This prevents insight getter functions
+ * from returning the insight when using default parameters. It does not delete the insight from storage.
+ *
+ * From the user's perspective, soft-deleted insights will not be used in assistant responses but will still exist in storage.
+ *
+ * @param {string} insightId ID of the insight to soft-delete
+ * @returns {Promise<Insight|null>} The soft-deleted insight, or null if not found
+ */
+ static async softDeleteInsightById(insightId) {
+ return await InsightStore.softDeleteInsight(insightId);
+ }
+
+ /**
+ * Hard deletes an insight by its ID.
+ * Hard deletion permenantly removes the insight from storage entirely. This method should be used
+ * by UI to allow users to delete insights they no longer want stored.
+ *
+ * @param {string} insightId ID of the insight to hard-delete
+ * @returns {Promise<boolean>} True if the insight was found and deleted, false otherwise
+ */
+ static async hardDeleteInsightById(insightId) {
+ return await InsightStore.hardDeleteInsight(insightId);
+ }
+
+ /**
* Builds the prompt to classify a user message into insight categories and intents.
*
* @param {string} message User message to classify
diff --git a/browser/components/aiwindow/models/tests/xpcshell/test_InsightsManager.js b/browser/components/aiwindow/models/tests/xpcshell/test_InsightsManager.js
@@ -58,21 +58,22 @@ const ENDPOINT = "https://api.fake-endpoint.com/v1";
const MODEL = "fake-model";
/**
- * Helper function bulk-add insights
+ * Helper function to delete all insights before and after a test
*/
-async function addInsights() {
- for (const insight of TEST_INSIGHTS) {
- await InsightStore.addInsight(insight);
+async function deleteAllInsights() {
+ const insights = await InsightStore.getInsights({ includeSoftDeleted: true });
+ for (const insight of insights) {
+ await InsightStore.hardDeleteInsight(insight.id);
}
}
/**
- * Helper function to delete all insights after a test
+ * Helper function to bulk-add insights
*/
-async function deleteAllInsights() {
- const insights = await InsightStore.getInsights();
- for (const insight of insights) {
- await InsightStore.hardDeleteInsight(insight.id);
+async function addInsights() {
+ await deleteAllInsights();
+ for (const insight of TEST_INSIGHTS) {
+ await InsightStore.addInsight(insight);
}
}
@@ -182,6 +183,173 @@ add_task(async function test_getAllInsights() {
});
/**
+ * Tests soft deleting an insight by ID
+ */
+add_task(async function test_softDeleteInsightById() {
+ await addInsights();
+
+ // Pull insights that aren't already soft deleted
+ const insightsBeforeSoftDelete = await InsightsManager.getAllInsights();
+
+ // Pick an insight off the top to soft delete
+ const insightBeforeSoftDelete = insightsBeforeSoftDelete[0];
+
+ // Double check that the insight isn't already soft deleted
+ Assert.equal(
+ insightBeforeSoftDelete.is_deleted,
+ false,
+ "Insight should not be soft deleted initially."
+ );
+
+ // Soft delete the insight
+ const insightAfterSoftDelete = await InsightsManager.softDeleteInsightById(
+ insightBeforeSoftDelete.id
+ );
+
+ // Check that the insight is soft deleted
+ Assert.equal(
+ insightAfterSoftDelete.is_deleted,
+ true,
+ "Insight should be soft deleted after calling softDeleteInsightById."
+ );
+
+ // Retrieve all insights again, including soft deleted ones this time to make sure the deletion saved correctly
+ const insightsAfterSoftDelete = await InsightsManager.getAllInsights({
+ includeSoftDeleted: true,
+ });
+ const softDeletedInsights = insightsAfterSoftDelete.filter(
+ insight => insight.is_deleted
+ );
+ Assert.equal(
+ softDeletedInsights.length,
+ 1,
+ "There should be one soft deleted insight."
+ );
+
+ await deleteAllInsights();
+});
+
+/**
+ * Tests attempting to soft delete an insight that doesn't exist by ID
+ */
+add_task(async function test_softDeleteInsightById_not_found() {
+ await addInsights();
+
+ // Retrieve all insights, including soft deleted ones
+ const insightsBeforeSoftDelete = await InsightsManager.getAllInsights({
+ includeSoftDeleted: true,
+ });
+
+ // Check that no insights are soft deleted initially
+ const softDeletedInsightsBefore = insightsBeforeSoftDelete.filter(
+ insight => insight.is_deleted
+ );
+ Assert.equal(
+ softDeletedInsightsBefore.length,
+ 0,
+ "There should be no soft deleted insights initially."
+ );
+
+ // Attempt to soft delete a non-existent insight
+ const insightAfterSoftDelete =
+ await InsightsManager.softDeleteInsightById("non-existent-id");
+
+ // Check that the result is null (no insights were soft deleted)
+ Assert.equal(
+ insightAfterSoftDelete,
+ null,
+ "softDeleteInsightById should return null for non-existent insight ID."
+ );
+
+ // Retrieve all insights again to confirm no insights were soft deleted
+ const insightsAfterSoftDelete = await InsightsManager.getAllInsights({
+ includeSoftDeleted: true,
+ });
+ const softDeletedInsightsAfter = insightsAfterSoftDelete.filter(
+ insight => insight.is_deleted
+ );
+ Assert.equal(
+ softDeletedInsightsAfter.length,
+ 0,
+ "There should be no soft deleted insights after attempting to delete a non-existent insight."
+ );
+
+ await deleteAllInsights();
+});
+
+/**
+ * Tests hard deleting an insight by ID
+ */
+add_task(async function test_hardDeleteInsightById() {
+ await addInsights();
+
+ // Retrieve all insights, including soft deleted ones
+ const insightsBeforeHardDelete = await InsightsManager.getAllInsights({
+ includeSoftDeleted: true,
+ });
+
+ // Pick an insight off the top to test hard deletion
+ const insightBeforeHardDelete = insightsBeforeHardDelete[0];
+
+ // Hard delete the insight
+ const deletionResult = await InsightsManager.hardDeleteInsightById(
+ insightBeforeHardDelete.id
+ );
+
+ // Check that the deletion was successful
+ Assert.ok(
+ deletionResult,
+ "hardDeleteInsightById should return true on successful deletion."
+ );
+
+ // Retrieve all insights again to confirm the hard deletion was saved correctly
+ const insightsAfterHardDelete = await InsightsManager.getAllInsights({
+ includeSoftDeleted: true,
+ });
+ Assert.equal(
+ insightsAfterHardDelete.length,
+ insightsBeforeHardDelete.length - 1,
+ "There should be one fewer insight after hard deletion."
+ );
+
+ await deleteAllInsights();
+});
+
+/**
+ * Tests attempting to hard delete an insight that doesn't exist by ID
+ */
+add_task(async function test_hardDeleteInsightById_not_found() {
+ await addInsights();
+
+ // Retrieve all insights, including soft deleted ones
+ const insightsBeforeHardDelete = await InsightsManager.getAllInsights({
+ includeSoftDeleted: true,
+ });
+
+ // Hard delete the insight
+ const deletionResult =
+ await InsightsManager.hardDeleteInsightById("non-existent-id");
+
+ // Check that the result is false (no insights were hard deleted)
+ Assert.ok(
+ !deletionResult,
+ "hardDeleteInsightById should return false for non-existent insight ID."
+ );
+
+ // Retrieve all insights again to make sure no insights were hard deleted
+ const insightsAfterHardDelete = await InsightsManager.getAllInsights({
+ includeSoftDeleted: true,
+ });
+ Assert.equal(
+ insightsAfterHardDelete.length,
+ insightsBeforeHardDelete.length,
+ "Insight count before and after failed hard deletion should be the same."
+ );
+
+ await deleteAllInsights();
+});
+
+/**
* Tests building the message insight classification prompt
*/
add_task(async function test_buildMessageInsightClassificationPrompt() {
diff --git a/browser/components/aiwindow/services/InsightStore.sys.mjs b/browser/components/aiwindow/services/InsightStore.sys.mjs
@@ -279,13 +279,22 @@ export const InsightStore = {
* Field to sort by.
* @param {"asc"|"desc"} [options.sortDir="desc"]
* Sort direction.
+ * @param {boolean} [options.includeSoftDeleted=false]
+ * Whether to include soft-deleted insights.
* @returns {Promise<Insight[]>}
*/
- async getInsights({ sortBy = "updated_at", sortDir = "desc" } = {}) {
+ async getInsights({
+ sortBy = "updated_at",
+ sortDir = "desc",
+ includeSoftDeleted = false,
+ } = {}) {
await this.ensureInitialized();
let res = gState.insights;
- res = res.filter(i => !i.is_deleted);
+
+ if (!includeSoftDeleted) {
+ res = res.filter(i => !i.is_deleted);
+ }
if (sortBy) {
res = [...res].sort((a, b) => {
