diff --git a/src/renderer/src/components/browser-pane/annotate/browser-annotation-limit-message.ts b/src/renderer/src/components/browser-pane/annotate/browser-annotation-limit-message.ts new file mode 100644 index 00000000000..e03e723f907 --- /dev/null +++ b/src/renderer/src/components/browser-pane/annotate/browser-annotation-limit-message.ts @@ -0,0 +1,9 @@ +import { translate } from '@/i18n/i18n' +import { GRAB_BUDGET } from '../../../../../shared/browser-grab-types' + +export const browserAnnotationLimitMessage = (): string => + translate( + 'auto.components.browser.pane.annotate.use.browser.page.grab.annotations.limitReached', + 'Limit of {{limit}} annotations reached. Send, delete or clear some to add more.', + { limit: GRAB_BUDGET.annotationsMaxPerPage } + ) diff --git a/src/renderer/src/components/browser-pane/annotate/use-browser-page-grab-annotations.test.tsx b/src/renderer/src/components/browser-pane/annotate/use-browser-page-grab-annotations.test.tsx index 1560bc39f90..1c143898d00 100644 --- a/src/renderer/src/components/browser-pane/annotate/use-browser-page-grab-annotations.test.tsx +++ b/src/renderer/src/components/browser-pane/annotate/use-browser-page-grab-annotations.test.tsx @@ -4,9 +4,10 @@ import { act, cleanup, renderHook, waitFor } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { createTestStore } from '@/store/slices/browser-slice-test-harness' import { makeAnnotation } from '@/store/slices/browser-annotation-test-fixture' -import type { - BrowserGrabResult, - BrowserCaptureSelectionScreenshotResult +import { + GRAB_BUDGET, + type BrowserGrabResult, + type BrowserCaptureSelectionScreenshotResult } from '../../../../../shared/browser-grab-types' import { useGrabMode } from './useGrabMode' import { useBrowserPageGrabAnnotations } from './use-browser-page-grab-annotations' @@ -40,7 +41,7 @@ afterEach(() => { Reflect.deleteProperty(window, 'api') }) -function mount(invalidateBeforePendingEffect = false) { +function mount(invalidateBeforePendingEffect = false, browserAnnotationsLength = 0) { const selection = deferred() const screenshot = deferred() const awaitGrabSelection = vi @@ -72,7 +73,7 @@ function mount(invalidateBeforePendingEffect = false) { containerRef, webviewRef, setBrowserOverlayViewport, - browserAnnotationsLength: 0 + browserAnnotationsLength }) const { cancelPendingBrowserCapture } = annotations useLayoutEffect(() => { @@ -187,3 +188,31 @@ describe('capture cancellation on a document boundary', () => { ]) }) }) + +describe('per-page annotation limit', () => { + it('tells the user the page is full instead of opening a draft that cannot be saved', async () => { + const h = mount(false, GRAB_BUDGET.annotationsMaxPerPage) + act(() => h.result.current.annotations.startGrabIntent('annotate')) + await waitFor(() => expect(h.awaitGrabSelection).toHaveBeenCalledOnce()) + await act(async () => { + h.selection.resolve(selected()) + h.screenshot.resolve({ ok: false, reason: 'fixture' }) + await h.screenshot.promise + }) + await waitFor(() => expect(h.result.current.annotations.grabToast?.type).toBe('error')) + expect(h.result.current.annotations.pendingAnnotationPayload).toBeNull() + }) + + it('shows no limit toast for a selection that loading cancelled first', async () => { + const h = mount(true, GRAB_BUDGET.annotationsMaxPerPage) + act(() => h.result.current.annotations.startGrabIntent('annotate')) + await waitFor(() => expect(h.awaitGrabSelection).toHaveBeenCalledOnce()) + await act(async () => { + h.selection.resolve(selected()) + h.screenshot.resolve({ ok: false, reason: 'fixture' }) + await h.screenshot.promise + }) + expect(h.result.current.grab.state).toBe('idle') + expect(h.result.current.annotations.grabToast).toBeNull() + }) +}) diff --git a/src/renderer/src/components/browser-pane/annotate/use-browser-page-grab-annotations.ts b/src/renderer/src/components/browser-pane/annotate/use-browser-page-grab-annotations.ts index 86ade1e4450..5e6649ffa69 100644 --- a/src/renderer/src/components/browser-pane/annotate/use-browser-page-grab-annotations.ts +++ b/src/renderer/src/components/browser-pane/annotate/use-browser-page-grab-annotations.ts @@ -11,11 +11,13 @@ import { import { translate } from '@/i18n/i18n' import { useMountedRef } from '@/hooks/useMountedRef' import { useAppStore } from '@/store' -import type { - BrowserAnnotationIntent, - BrowserGrabPayload +import { + isBrowserAnnotationPageFull, + type BrowserAnnotationIntent, + type BrowserGrabPayload } from '../../../../../shared/browser-grab-types' import { formatGrabPayloadAsText } from './GrabConfirmationSheet' +import { browserAnnotationLimitMessage } from './browser-annotation-limit-message' import { createBrowserAnnotationId, createBrowserAnnotationPayload, @@ -85,14 +87,16 @@ export function useBrowserPageGrabAnnotations({ const [grabToast, setGrabToast] = useState(null) const grabRef = useRef(grab) const grabPayloadRef = useRef(grab.payload) + const browserAnnotationsLengthRef = useRef(browserAnnotationsLength) useLayoutEffect(() => { + browserAnnotationsLengthRef.current = browserAnnotationsLength toolTargetIdRef.current = toolTargetId grabIntentRef.current = grabIntent pendingAnnotationPayloadRef.current = pendingAnnotationPayload grabRef.current = grab grabPayloadRef.current = grab.payload - }, [grab, grabIntent, pendingAnnotationPayload, toolTargetId]) + }, [browserAnnotationsLength, grab, grabIntent, pendingAnnotationPayload, toolTargetId]) // Why: Radix fires onOpenChange(false) before onSelect, so this flag lets onOpenChange skip the rearm that would clear the payload first. const grabMenuActionTakenRef = useRef(false) const recordFeatureInteraction = useAppStore((s) => s.recordFeatureInteraction) @@ -150,6 +154,14 @@ export function useBrowserPageGrabAnnotations({ return } if (grabIntent === 'annotate') { + // The store refuses notes past the cap, so say so before the user writes one. + if ( + grabPayloadRef.current === grab.payload && + isBrowserAnnotationPageFull(browserAnnotationsLengthRef.current) + ) { + showGrabToast(browserAnnotationLimitMessage(), 'error', grab.payload) + return + } // Loading may cancel the selection before React accepts this pending snapshot. setPendingAnnotationPayload(() => grabPayloadRef.current === grab.payload ? grab.payload : null diff --git a/src/renderer/src/components/browser-pane/assemble-chrome/browser-page-chrome-banners.tsx b/src/renderer/src/components/browser-pane/assemble-chrome/browser-page-chrome-banners.tsx index 5bced58139a..94428245bf2 100644 --- a/src/renderer/src/components/browser-pane/assemble-chrome/browser-page-chrome-banners.tsx +++ b/src/renderer/src/components/browser-pane/assemble-chrome/browser-page-chrome-banners.tsx @@ -10,7 +10,11 @@ import { import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' import { translate } from '@/i18n/i18n' import { BrowserAnnotationSendMenuContent } from '../annotate/BrowserAnnotationSendMenuContent' -import type { BrowserGrabPayload } from '../../../../../shared/browser-grab-types' +import { browserAnnotationLimitMessage } from '../annotate/browser-annotation-limit-message' +import { + isBrowserAnnotationPageFull, + type BrowserGrabPayload +} from '../../../../../shared/browser-grab-types' import type { GrabModeHook } from '../annotate/useGrabMode' import { preventAgentSendTargetOutsideDismiss } from '../annotate/prevent-agent-send-target-outside-dismiss' import type { GrabIntent } from '../describe-page/browser-page-types' @@ -97,22 +101,24 @@ export function BrowserPageChromeBanners({ 'auto.components.browser.pane.BrowserPane.b733a91bd9', 'Add feedback for the selected element.' ) - : browserAnnotationsLength === 1 - ? translate( - 'auto.components.browser.pane.BrowserPane.074f0ed10b', - '{{value0}} annotation ready. Select another element or copy all feedback.', - { value0: browserAnnotationsLength } - ) - : browserAnnotationsLength > 0 + : isBrowserAnnotationPageFull(browserAnnotationsLength) + ? browserAnnotationLimitMessage() + : browserAnnotationsLength === 1 ? translate( - 'auto.components.browser.pane.BrowserPane.a2164a6e5a', - '{{value0}} annotations ready. Select another element or copy all feedback.', + 'auto.components.browser.pane.BrowserPane.074f0ed10b', + '{{value0}} annotation ready. Select another element or copy all feedback.', { value0: browserAnnotationsLength } ) - : translate( - 'auto.components.browser.pane.BrowserPane.777b5bc4ec', - 'Click an element to add feedback for the agent.' - ) + : browserAnnotationsLength > 0 + ? translate( + 'auto.components.browser.pane.BrowserPane.a2164a6e5a', + '{{value0}} annotations ready. Select another element or copy all feedback.', + { value0: browserAnnotationsLength } + ) + : translate( + 'auto.components.browser.pane.BrowserPane.777b5bc4ec', + 'Click an element to add feedback for the agent.' + ) : grab.state === 'confirming' ? translate( 'auto.components.browser.pane.BrowserPane.e852e20cea', diff --git a/src/renderer/src/i18n/locales/de.json b/src/renderer/src/i18n/locales/de.json index 1729332553f..d8d17bdb0bf 100644 --- a/src/renderer/src/i18n/locales/de.json +++ b/src/renderer/src/i18n/locales/de.json @@ -16955,7 +16955,8 @@ "annotations": { "0c7b9b2b7a": "Kopiert", "c937229f19": "Screenshot erstellt", - "1f5cb19034": "Anmerkung hinzugefügt" + "1f5cb19034": "Anmerkung hinzugefügt", + "limitReached": "Limit von {{limit}} Anmerkungen erreicht. Senden, entfernen oder löschen Sie einige, um weitere hinzuzufügen." } } } diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 114b17551ba..94cab208a93 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -17382,7 +17382,8 @@ "annotations": { "0c7b9b2b7a": "Copied", "c937229f19": "Screenshotted", - "1f5cb19034": "Annotation added" + "1f5cb19034": "Annotation added", + "limitReached": "Limit of {{limit}} annotations reached. Send, delete or clear some to add more." } } } diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index 1a9272ecf80..451bfe40751 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -16976,7 +16976,8 @@ "annotations": { "0c7b9b2b7a": "Copiado", "c937229f19": "Captura tomada", - "1f5cb19034": "Anotación añadida" + "1f5cb19034": "Anotación añadida", + "limitReached": "Se alcanzó el límite de {{limit}} anotaciones. Envíe, elimine o borre algunas para añadir más." } } } diff --git a/src/renderer/src/i18n/locales/fr.json b/src/renderer/src/i18n/locales/fr.json index 94c5be9d497..e921f3fa08a 100644 --- a/src/renderer/src/i18n/locales/fr.json +++ b/src/renderer/src/i18n/locales/fr.json @@ -17200,7 +17200,8 @@ "annotations": { "0c7b9b2b7a": "Copied", "c937229f19": "Capture effectuée", - "1f5cb19034": "Annotation ajoutée" + "1f5cb19034": "Annotation ajoutée", + "limitReached": "Limite de {{limit}} annotations atteinte. Envoyez, supprimez ou effacez-en pour en ajouter d’autres." } } } diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index 3ee4850ff73..3507c749b66 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -16572,7 +16572,8 @@ "annotations": { "0c7b9b2b7a": "コピーしました", "c937229f19": "スクリーンショット", - "1f5cb19034": "注釈を追加しました" + "1f5cb19034": "注釈を追加しました", + "limitReached": "注釈は {{limit}} 件までです。追加するには、いくつか送信、削除、またはクリアしてください。" } } } diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index f0a3546f500..5ad48ebda75 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -16572,7 +16572,8 @@ "annotations": { "0c7b9b2b7a": "복사됨", "c937229f19": "스크린샷됨", - "1f5cb19034": "주석이 추가됨" + "1f5cb19034": "주석이 추가됨", + "limitReached": "주석은 최대 {{limit}}개까지 추가할 수 있습니다. 더 추가하려면 일부를 보내거나 삭제하거나 지우세요." } } } diff --git a/src/renderer/src/i18n/locales/pt.json b/src/renderer/src/i18n/locales/pt.json index 8a14319b488..b12c098646a 100644 --- a/src/renderer/src/i18n/locales/pt.json +++ b/src/renderer/src/i18n/locales/pt.json @@ -17018,7 +17018,8 @@ "annotations": { "0c7b9b2b7a": "Copiado", "c937229f19": "Capturado", - "1f5cb19034": "Anotação adicionada" + "1f5cb19034": "Anotação adicionada", + "limitReached": "Limite de {{limit}} anotações atingido. Envie, exclua ou limpe algumas para adicionar mais." } } } diff --git a/src/renderer/src/i18n/locales/ru.json b/src/renderer/src/i18n/locales/ru.json index 1ba78de3334..252732b3394 100644 --- a/src/renderer/src/i18n/locales/ru.json +++ b/src/renderer/src/i18n/locales/ru.json @@ -17457,7 +17457,8 @@ "annotations": { "0c7b9b2b7a": "Скопировано", "c937229f19": "Снимок сделан", - "1f5cb19034": "Заметка добавлена" + "1f5cb19034": "Заметка добавлена", + "limitReached": "Достигнут лимит в {{limit}} аннотаций. Отправьте, удалите или очистите часть из них, чтобы добавить новые." } } } diff --git a/src/renderer/src/i18n/locales/tr.json b/src/renderer/src/i18n/locales/tr.json index b34cda27e92..0a52cdbabee 100644 --- a/src/renderer/src/i18n/locales/tr.json +++ b/src/renderer/src/i18n/locales/tr.json @@ -16941,7 +16941,8 @@ "annotations": { "0c7b9b2b7a": "Kopyalandı", "c937229f19": "Ekran görüntüsü alındı", - "1f5cb19034": "Ek açıklama eklendi" + "1f5cb19034": "Ek açıklama eklendi", + "limitReached": "{{limit}} ek açıklama sınırına ulaşıldı. Daha fazla eklemek için bazılarını gönderin, silin veya temizleyin." } } } diff --git a/src/renderer/src/i18n/locales/zh-Hant.json b/src/renderer/src/i18n/locales/zh-Hant.json index af390fb6a01..58b6613e770 100644 --- a/src/renderer/src/i18n/locales/zh-Hant.json +++ b/src/renderer/src/i18n/locales/zh-Hant.json @@ -16727,7 +16727,8 @@ "annotations": { "0c7b9b2b7a": "已複製", "c937229f19": "已截圖", - "1f5cb19034": "新增了註解" + "1f5cb19034": "新增了註解", + "limitReached": "已達到 {{limit}} 則註解的上限。請傳送、刪除或清除一些後再新增。" } } } diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index 5183ef18a04..85bf5847276 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -16572,7 +16572,8 @@ "annotations": { "0c7b9b2b7a": "已复制", "c937229f19": "截图", - "1f5cb19034": "添加了注释" + "1f5cb19034": "添加了注释", + "limitReached": "已达到 {{limit}} 条注释的上限。请发送、删除或清除一些后再添加。" } } } diff --git a/src/renderer/src/store/slices/browser-annotation-retention.test.ts b/src/renderer/src/store/slices/browser-annotation-retention.test.ts index d4962ce2165..2871d01a5af 100644 --- a/src/renderer/src/store/slices/browser-annotation-retention.test.ts +++ b/src/renderer/src/store/slices/browser-annotation-retention.test.ts @@ -1,5 +1,4 @@ import { afterEach, describe, expect, it, vi } from 'vitest' -import { GRAB_BUDGET } from '../../../../shared/browser-grab-types' import { ORCA_LAYOUT_OWNER_IS_MAIN } from '../../../../shared/layout-owner-switch' import { layoutApplyTestTransport } from '@/runtime/layout-apply-test-transport.test-fixture' import { createBrowserMockApi } from './browser-slice-test-harness' @@ -60,39 +59,19 @@ describe('saved browser note lifetime and marker eligibility', () => { expect(store.getState().browserAnnotationMarkerIdsByPageId[pageId]).toEqual(['fresh']) }) - it('shares the 20-note budget across URLs and bounds marker IDs to retained notes', async () => { + it('prunes marker IDs when notes are deleted or cleared', async () => { const { store, pageId } = await createAnnotatedPage() - for (let index = 0; index < GRAB_BUDGET.annotationsMaxPerPage + 3; index++) { - const url = `https://example.com/page-${index % 2}` - store.getState().setBrowserPageUrl(pageId, url) - store.getState().invalidateBrowserPageAnnotationGeometry(pageId) - const note = makeAnnotation(pageId, `note-${index}`) - note.payload.page.sanitizedUrl = url - store.getState().addBrowserPageAnnotation(note) - } - const saved = store.getState().browserAnnotationsByPageId[pageId] - const eligible = store.getState().browserAnnotationMarkerIdsByPageId[pageId] - expect(saved).toHaveLength(20) - expect(saved[0].id).toBe('note-3') - expect(new Set(saved.map((note) => note.payload.page.sanitizedUrl)).size).toBe(2) - expect(eligible).toEqual(['note-22']) - expect(eligible.every((id) => saved.some((note) => note.id === id))).toBe(true) + store.getState().addBrowserPageAnnotation(makeAnnotation(pageId, 'second')) + expect(store.getState().browserAnnotationMarkerIdsByPageId[pageId]).toEqual([ + 'annotation-1', + 'second' + ]) - store.getState().deleteBrowserPageAnnotation(pageId, 'note-22') - expect(store.getState().browserAnnotationMarkerIdsByPageId[pageId]).toBeUndefined() + store.getState().deleteBrowserPageAnnotation(pageId, 'second') + expect(store.getState().browserAnnotationMarkerIdsByPageId[pageId]).toEqual(['annotation-1']) store.getState().clearBrowserPageAnnotations(pageId) expect(store.getState().browserAnnotationsByPageId[pageId]).toBeUndefined() - }) - - it('prunes eligible IDs when the oldest current-document notes are evicted', async () => { - const { store, pageId } = await createAnnotatedPage() - for (let index = 0; index < 22; index++) { - store.getState().addBrowserPageAnnotation(makeAnnotation(pageId, `note-${index}`)) - } - expect(store.getState().browserAnnotationMarkerIdsByPageId[pageId]).toEqual( - store.getState().browserAnnotationsByPageId[pageId].map((note) => note.id) - ) - expect(store.getState().browserAnnotationMarkerIdsByPageId[pageId]).toHaveLength(20) + expect(store.getState().browserAnnotationMarkerIdsByPageId[pageId]).toBeUndefined() }) it('removes an accepted delivery snapshot after navigation while preserving later edits and additions', async () => { diff --git a/src/renderer/src/store/slices/browser.test.ts b/src/renderer/src/store/slices/browser.test.ts index 0eb992ed801..f4b6413004c 100644 --- a/src/renderer/src/store/slices/browser.test.ts +++ b/src/renderer/src/store/slices/browser.test.ts @@ -611,7 +611,7 @@ describe('createBrowserSlice annotations', () => { expect(store.getState().setTabLabel).not.toHaveBeenCalled() }) - it('caps stored browser annotations per page', async () => { + it('refuses browser annotations past the per-page cap instead of dropping the oldest', async () => { const { store, main } = await createTestStoreDrawnFromMain() const tab = store.getState().createBrowserTab('wt-1', 'https://example.com') await main.settle() @@ -626,7 +626,8 @@ describe('createBrowserSlice annotations', () => { const annotations = store.getState().browserAnnotationsByPageId[pageId] ?? [] expect(annotations).toHaveLength(GRAB_BUDGET.annotationsMaxPerPage) - expect(annotations[0]?.id).toBe('annotation-3') + expect(annotations[0]?.id).toBe('annotation-0') + expect(annotations.at(-1)?.id).toBe(`annotation-${GRAB_BUDGET.annotationsMaxPerPage - 1}`) }) it('sanitizes persistent annotation payloads at the store boundary', async () => { diff --git a/src/renderer/src/store/slices/browser/browser-page-metadata-actions.ts b/src/renderer/src/store/slices/browser/browser-page-metadata-actions.ts index 48ef5426846..c2f8f7bf1e6 100644 --- a/src/renderer/src/store/slices/browser/browser-page-metadata-actions.ts +++ b/src/renderer/src/store/slices/browser/browser-page-metadata-actions.ts @@ -1,4 +1,4 @@ -import { GRAB_BUDGET } from '../../../../../shared/browser-grab-types' +import { isBrowserAnnotationPageFull } from '../../../../../shared/browser-grab-types' import type { BrowserSlice, BrowserSliceGet, @@ -79,9 +79,11 @@ export function createBrowserPageMetadataActions( addBrowserPageAnnotation: (annotation) => set((s) => { const existing = s.browserAnnotationsByPageId[annotation.browserPageId] ?? [] - const next = [...existing, sanitizeBrowserPageAnnotation(annotation)].slice( - -GRAB_BUDGET.annotationsMaxPerPage - ) + // A full page refuses new notes; evicting the oldest would lose feedback unseen. + if (isBrowserAnnotationPageFull(existing.length)) { + return s + } + const next = [...existing, sanitizeBrowserPageAnnotation(annotation)] return { browserAnnotationsByPageId: { ...s.browserAnnotationsByPageId, diff --git a/src/shared/browser-grab-types.ts b/src/shared/browser-grab-types.ts index f4f3c86c37a..82af8b9ab14 100644 --- a/src/shared/browser-grab-types.ts +++ b/src/shared/browser-grab-types.ts @@ -179,6 +179,10 @@ export const GRAB_BUDGET = { screenshotMaxBytes: 2 * 1024 * 1024 } as const +export function isBrowserAnnotationPageFull(annotationCount: number): boolean { + return annotationCount >= GRAB_BUDGET.annotationsMaxPerPage +} + /** Only these attribute names are included in the payload by default. */ export const GRAB_SAFE_ATTRIBUTE_NAMES = new Set([ 'id',