AI document editing in HTML, with reviewable and undoable changes

[urjitc]

Switches the AI's view of workspace documents from Markdown to HTML, and adds a way to see and undo what it changed.

Why

Markdown was a lossy projection: tables, math and task lists did not survive the round trip, and "change this paragraph" meant matching on text. Documents were never stored as Markdown — it was only what the model saw — so this changes the projection, not the storage. There is nothing to migrate.

What the model sees

Documents are read as HTML blocks, each top-level block carrying a data-ref. A ref is an id plus a fingerprint of that block's content, so if a human edits a paragraph between the model reading it and editing it, the edit is rejected (stale_target) rather than clobbering the change.

Edits are structural: replace, insert_before, insert_after, delete, and replace_all. Content that is not HTML — Markdown, most often — is rejected rather than flattened into a paragraph of literal source.

What the reader sees

Every turn that edited documents gets a receipt in chat: one row per document with a Lines: +N −M tally. Clicking a row opens the document and marks the changes inline — insertions underlined, deletions struck through — with the toolbar becoming Reviewing changes: Undo | Done. Review is a read-only mode, and undo asks for confirmation.

Rows persist for the life of the transcript. Whether changes can still be reviewed comes and goes — editing the document yourself closes that window — so the row keeps the record and reports its state (Edited since, Undone).

Design notes for review

• Review is keyed to the document, not to a view. Keying it to a view instance made every "where is it now" question a lifecycle problem, and StrictMode's teardown wiped reviews as the document mounted.
• Marks are derived, not mapped. The plugin stores the before-document and recomputes decorations against whatever is on screen, so a document that arrives late is marked correctly instead of not at all.
• Staleness is the server's call, and is not cached. The document session compares the live document to the receipt; caching that verdict once made a reader's own later writing show up as the assistant's.
• Only the newest receipts are kept. Undo works on the latest edit alone, so older whole-document snapshots were unusable the moment they were superseded.
• linkedom is required: @tiptap/core's generateJSON/generateHTML throw no window object in workerd. Verified, and it stays out of the client bundle.

Verification

vp check, 218 unit tests, 32 workerd tests, and a production build all pass. A document exercising the full schema — headings, all marks, links, both list types, task lists, code blocks, tables, alignment, inline and block math — round-trips through the AI contract without loss.

Known and deliberate

• A deleted document on an evicted Durable Object can make its status request error rather than answer not_found. The card skips deleted items client-side so it is not reachable from the UI; the server path is left alone because the obvious patch risks checkpointing an empty document over real content.
• document-markdown.ts remains, used by search indexing and file import. Retiring it is independent of this change.

🤖 Generated with Claude Code

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

Summary by CodeRabbit

• New Features

  • AI-assisted document editing now supports structured HTML edits, citation preservation, and reliable edit tracking.
  • Review AI changes directly in documents, with highlighted additions, removals, and changed blocks.
  • Undo applied AI edits from chat or the document toolbar.
  • Chat responses show edited documents, paths, and line-change summaries with review links.
  • Workspace reads now provide HTML content with block-based references.
  • Added support for Heading 4 and improved citation/link interactions.

• Bug Fixes

  • Improved handling of partial, conflicting, stale, and no-op document edits.
  • Enhanced citation resolution and document content validation.

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[cubic-dev-ai]

9 issues found and verified against the latest diff

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/features/workspaces/documents/use-document-edit-review-overlay.ts">

<violation number="1" location="src/features/workspaces/documents/use-document-edit-review-overlay.ts:14">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

This new hook implements core review/undo overlay behavior (loading receipts, toasting unavailable statuses, toggling editor read-only mode, and cleaning up on unmount), but no tests exist for it anywhere in the PR. The custom rule flags behavior-change PRs where changed behavior is not exercised by tests when regression-style assertions are practical. Consider adding tests that verify toast messages per unavailable status, `showDocumentEditReview`/`hideDocumentEditReview` calls, and `editor.setEditable` toggle behavior on mount/unmount.</violation>
</file>

<file name="src/features/workspaces/documents/document-edit-review-queries.ts">

<violation number="1" location="src/features/workspaces/documents/document-edit-review-queries.ts:24">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The comment claims this query is 'deliberately not cached' to avoid showing a stale verdict, but only `staleTime: 0` is set. In TanStack Query, `staleTime: 0` marks data as immediately stale (triggering background refetches) but does **not** remove it from the cache; inactive queries are still retained for the default `gcTime` (5 minutes). Reopening the review within that window can still briefly display the old, incorrect verdict while refetching. If truly uncached behavior is intended—as both the comment and PR description state—`gcTime: 0` should be added alongside `staleTime: 0` so the query is garbage-collected immediately when inactive.</violation>
</file>

<file name="src/features/workspaces/documents/tiptap-schema.ts">

<violation number="1" location="src/features/workspaces/documents/tiptap-schema.ts:78">
P1: Custom agent: **Flag AI Slop and Fabricated Changes**

The Link extension's `openOnClick` option was silently flipped from `false` to `true` in the shared document schema. This changes live-editor behavior so that clicking a link navigates away, yet the PR description does not mention this change and there are no tests exercising it. Revert the flip or add an explicit justification and a regression-style test if the change is intentional.</violation>
</file>

<file name="src/features/workspaces/components/ai-chat/AiChatMessageRow.tsx">

<violation number="1" location="src/features/workspaces/components/ai-chat/AiChatMessageRow.tsx:62">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The new document-edit receipt rendering and its streaming/content gating (`isAssistant && display?.kind === "content" && !isStreaming`) are not exercised by any tests. For a behavior-change PR of this scope, regression-style assertions — ensuring receipts appear for non-streaming assistant content, are hidden while streaming, and that the grouping logic correctly aggregates line changes — are practical and should be present. Consider adding unit tests for `getAiChatDocumentEditGroups` and component/render tests for `AiChatDocumentEditActions` to prevent silent regressions in this new UI path.</violation>
</file>

<file name="src/features/workspaces/documents/document-edit-review-extension.ts">

<violation number="1" location="src/features/workspaces/documents/document-edit-review-extension.ts:51">
P1: Custom agent: **Flag AI Slop and Fabricated Changes**

The new `document-edit-review-extension.ts` implements non-trivial review behavior (ChangeSet diffing, DecorationSet creation, show/hide state transitions, and deletion widgets) but appears to have no accompanying automated tests. Rule 1 flags behavior changes that are not exercised by tests, especially when regression-style assertions are practical. Consider adding unit tests for the plugin state transitions, decoration generation, and widget fallback behavior to validate this core review functionality.</violation>
</file>

<file name="src/features/workspaces/documents/use-document-edit-receipt-undo.ts">

<violation number="1" location="src/features/workspaces/documents/use-document-edit-receipt-undo.ts:24">
P1: Custom agent: **Flag AI Slop and Fabricated Changes**

The undo handler treats `result.status === "reverted"` as a success (showing "Changes undone."), but `"reverted"` is also defined in the `undoUnavailableMessages` map as "These changes were already undone." This makes that map entry unreachable dead code. More importantly, the server returns `"reverted"` both when an undo succeeds and when the receipt was already reverted (see `document-session.ts` lines 328 and 390), so a user requesting undo on an already-reverted edit will incorrectly see a success toast instead of an error. Consider introducing a distinct success status for the undo operation, or explicitly branching on whether `"reverted"` means success versus already-done so the correct message is shown.</violation>
</file>

<file name="src/features/workspaces/documents/document-html-chunk.ts">

<violation number="1" location="src/features/workspaces/documents/document-html-chunk.ts:41">
P2: Large document reads now wait for each block's serialization and SHA-256 digest serially, so a 48,000-character chunk containing thousands of ordinary paragraphs can incur thousands of async operations before the model receives any content. Batching or prefetching block serialization while preserving the size cutoff would avoid making read latency grow with one await per block.</violation>
</file>

<file name="src/features/workspaces/components/ai-chat/AiChatDocumentEditActions.tsx">

<violation number="1" location="src/features/workspaces/components/ai-chat/AiChatDocumentEditActions.tsx:32">
P3: Screen readers cannot reliably announce this receipt section's purpose because its `aria-label` is on a plain `div` with no semantic role. Adding `role="group"` would expose the label while preserving the current layout.</violation>
</file>

<file name="src/features/workspaces/documents/document-ai-edits.ts">

<violation number="1" location="src/features/workspaces/documents/document-ai-edits.ts:175">
P2: Using full `node.toJSON()` for atom line keys includes generated `aiRef`, so unchanged atoms can be counted as removed and added after ref regeneration. Building the key without `aiRef` keeps `lineChanges` stable for unchanged atom content.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P1: Custom agent: Flag AI Slop and Fabricated Changes

The Link extension's openOnClick option was silently flipped from false to true in the shared document schema. This changes live-editor behavior so that clicking a link navigates away, yet the PR description does not mention this change and there are no tests exercising it. Revert the flip or add an explicit justification and a regression-style test if the change is intentional.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/tiptap-schema.ts, line 78:

<comment>The Link extension's `openOnClick` option was silently flipped from `false` to `true` in the shared document schema. This changes live-editor behavior so that clicking a link navigates away, yet the PR description does not mention this change and there are no tests exercising it. Revert the flip or add an explicit justification and a regression-style test if the change is intentional.</comment>

<file context>
@@ -40,7 +75,7 @@ export function getTiptapDocumentSchemaExtensions({
 		Highlight,
 		Link.configure({
-			openOnClick: false,
+			openOnClick: true,
 			autolink: true,
 			defaultProtocol: "https",
</file context>

Suggested change

	openOnClick: true,
	openOnClick: false,

[cubic-dev-ai]

P1: Custom agent: Flag AI Slop and Fabricated Changes

The new document-edit-review-extension.ts implements non-trivial review behavior (ChangeSet diffing, DecorationSet creation, show/hide state transitions, and deletion widgets) but appears to have no accompanying automated tests. Rule 1 flags behavior changes that are not exercised by tests, especially when regression-style assertions are practical. Consider adding unit tests for the plugin state transitions, decoration generation, and widget fallback behavior to validate this core review functionality.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/document-edit-review-extension.ts, line 51:

<comment>The new `document-edit-review-extension.ts` implements non-trivial review behavior (ChangeSet diffing, DecorationSet creation, show/hide state transitions, and deletion widgets) but appears to have no accompanying automated tests. Rule 1 flags behavior changes that are not exercised by tests, especially when regression-style assertions are practical. Consider adding unit tests for the plugin state transitions, decoration generation, and widget fallback behavior to validate this core review functionality.</comment>

<file context>
@@ -0,0 +1,204 @@
+	},
+};
+
+export const DocumentEditReviewExtension = Extension.create({
+	name: "documentEditReview",
+
</file context>

[cubic-dev-ai]

P1: Custom agent: Flag AI Slop and Fabricated Changes

The undo handler treats result.status === "reverted" as a success (showing "Changes undone."), but "reverted" is also defined in the undoUnavailableMessages map as "These changes were already undone." This makes that map entry unreachable dead code. More importantly, the server returns "reverted" both when an undo succeeds and when the receipt was already reverted (see document-session.ts lines 328 and 390), so a user requesting undo on an already-reverted edit will incorrectly see a success toast instead of an error. Consider introducing a distinct success status for the undo operation, or explicitly branching on whether "reverted" means success versus already-done so the correct message is shown.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/use-document-edit-receipt-undo.ts, line 20:

<comment>The undo handler treats `result.status === "reverted"` as a success (showing "Changes undone."), but `"reverted"` is also defined in the `undoUnavailableMessages` map as "These changes were already undone." This makes that map entry unreachable dead code. More importantly, the server returns `"reverted"` both when an undo succeeds and when the receipt was already reverted (see `document-session.ts` lines 328 and 390), so a user requesting undo on an already-reverted edit will incorrectly see a success toast instead of an error. Consider introducing a distinct success status for the undo operation, or explicitly branching on whether `"reverted"` means success versus already-done so the correct message is shown.</comment>

<file context>
@@ -0,0 +1,43 @@
+	return useMutation({
+		mutationFn: () => undoDocumentEditReceiptFn({ data: target }),
+		onSuccess: async (result) => {
+			if (result.status === "reverted") {
+				hideReview();
+				toast.success("Changes undone.");
</file context>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

This new hook implements core review/undo overlay behavior (loading receipts, toasting unavailable statuses, toggling editor read-only mode, and cleaning up on unmount), but no tests exist for it anywhere in the PR. The custom rule flags behavior-change PRs where changed behavior is not exercised by tests when regression-style assertions are practical. Consider adding tests that verify toast messages per unavailable status, showDocumentEditReview/hideDocumentEditReview calls, and editor.setEditable toggle behavior on mount/unmount.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/use-document-edit-review-overlay.ts, line 14:

<comment>This new hook implements core review/undo overlay behavior (loading receipts, toasting unavailable statuses, toggling editor read-only mode, and cleaning up on unmount), but no tests exist for it anywhere in the PR. The custom rule flags behavior-change PRs where changed behavior is not exercised by tests when regression-style assertions are practical. Consider adding tests that verify toast messages per unavailable status, `showDocumentEditReview`/`hideDocumentEditReview` calls, and `editor.setEditable` toggle behavior on mount/unmount.</comment>

<file context>
@@ -0,0 +1,79 @@
+} from "#/features/workspaces/documents/document-edit-review-extension";
+import { documentEditReceiptReviewQueryOptions } from "#/features/workspaces/documents/document-edit-review-queries";
+
+export function useDocumentEditReviewOverlay({
+	canEdit,
+	editor,
</file context>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

The comment claims this query is 'deliberately not cached' to avoid showing a stale verdict, but only staleTime: 0 is set. In TanStack Query, staleTime: 0 marks data as immediately stale (triggering background refetches) but does not remove it from the cache; inactive queries are still retained for the default gcTime (5 minutes). Reopening the review within that window can still briefly display the old, incorrect verdict while refetching. If truly uncached behavior is intended—as both the comment and PR description state—gcTime: 0 should be added alongside staleTime: 0 so the query is garbage-collected immediately when inactive.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/document-edit-review-queries.ts, line 19:

<comment>The comment claims this query is 'deliberately not cached' to avoid showing a stale verdict, but only `staleTime: 0` is set. In TanStack Query, `staleTime: 0` marks data as immediately stale (triggering background refetches) but does **not** remove it from the cache; inactive queries are still retained for the default `gcTime` (5 minutes). Reopening the review within that window can still briefly display the old, incorrect verdict while refetching. If truly uncached behavior is intended—as both the comment and PR description state—`gcTime: 0` should be added alongside `staleTime: 0` so the query is garbage-collected immediately when inactive.</comment>

<file context>
@@ -0,0 +1,34 @@
+		// whether the document still matches the receipt, and a verdict cached
+		// from an earlier open would mark the reader's own later writing as the
+		// assistant's.
+		staleTime: 0,
+	});
+}
</file context>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

The new document-edit receipt rendering and its streaming/content gating (isAssistant && display?.kind === "content" && !isStreaming) are not exercised by any tests. For a behavior-change PR of this scope, regression-style assertions — ensuring receipts appear for non-streaming assistant content, are hidden while streaming, and that the grouping logic correctly aggregates line changes — are practical and should be present. Consider adding unit tests for getAiChatDocumentEditGroups and component/render tests for AiChatDocumentEditActions to prevent silent regressions in this new UI path.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/ai-chat/AiChatMessageRow.tsx, line 62:

<comment>The new document-edit receipt rendering and its streaming/content gating (`isAssistant && display?.kind === "content" && !isStreaming`) are not exercised by any tests. For a behavior-change PR of this scope, regression-style assertions — ensuring receipts appear for non-streaming assistant content, are hidden while streaming, and that the grouping logic correctly aggregates line changes — are practical and should be present. Consider adding unit tests for `getAiChatDocumentEditGroups` and component/render tests for `AiChatDocumentEditActions` to prevent silent regressions in this new UI path.</comment>

<file context>
@@ -57,6 +59,10 @@ export default function AiChatMessageRow({
 		? []
 		: displayableParts.filter((part) => !isAttachmentPart(part));
 	const copyableText = !isStreaming ? getCopyableMessageText(message) : "";
+	const documentEditGroups =
+		isAssistant && display?.kind === "content" && !isStreaming
+			? getAiChatDocumentEditGroups(display.parts)
</file context>

[cubic-dev-ai]

P2: Large document reads now wait for each block's serialization and SHA-256 digest serially, so a 48,000-character chunk containing thousands of ordinary paragraphs can incur thousands of async operations before the model receives any content. Batching or prefetching block serialization while preserving the size cutoff would avoid making read latency grow with one await per block.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/document-html-chunk.ts, line 41:

<comment>Large document reads now wait for each block's serialization and SHA-256 digest serially, so a 48,000-character chunk containing thousands of ordinary paragraphs can incur thousands of async operations before the model receives any content. Batching or prefetching block serialization while preserving the size cutoff would avoid making read latency grow with one await per block.</comment>

<file context>
@@ -0,0 +1,64 @@
+	let characters = 0;
+	let endOffset = offset;
+	while (endOffset < document.childCount) {
+		const block = await serializeTiptapNodeToAiHtml(document.child(endOffset));
+		const separatorCharacters = content.length > 0 ? 1 : 0;
+		if (
</file context>

[cubic-dev-ai]

P2: Using full node.toJSON() for atom line keys includes generated aiRef, so unchanged atoms can be counted as removed and added after ref regeneration. Building the key without aiRef keeps lineChanges stable for unchanged atom content.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/document-ai-edits.ts, line 175:

<comment>Using full `node.toJSON()` for atom line keys includes generated `aiRef`, so unchanged atoms can be counted as removed and added after ref regeneration. Building the key without `aiRef` keeps `lineChanges` stable for unchanged atom content.</comment>

<file context>
@@ -0,0 +1,302 @@
+			}
+			// A rule or a formula holds no text but still occupies a line.
+			if (node.isAtom) {
+				countLine(JSON.stringify(node.toJSON()));
+				return false;
+			}
</file context>

[cubic-dev-ai]

P3: Screen readers cannot reliably announce this receipt section's purpose because its aria-label is on a plain div with no semantic role. Adding role="group" would expose the label while preserving the current layout.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/ai-chat/AiChatDocumentEditActions.tsx, line 32:

<comment>Screen readers cannot reliably announce this receipt section's purpose because its `aria-label` is on a plain `div` with no semantic role. Adding `role="group"` would expose the label while preserving the current layout.</comment>

<file context>
@@ -0,0 +1,107 @@
+
+	return (
+		<div
+			aria-label="Document changes from this response"
+			className="overflow-hidden rounded-lg bg-muted/40"
+		>
</file context>

[cubic-dev-ai]

14 issues found across 52 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/features/workspaces/components/document-editor/DocumentEditorSurface.tsx">

<violation number="1" location="src/features/workspaces/components/document-editor/DocumentEditorSurface.tsx:139">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

This PR wires up new review/undo behavior (`DocumentEditReviewExtension` and `useDocumentEditReviewOverlay`) but no tests in the codebase exercise these additions. The existing `document-ai-edits.test.ts` covers the core edit-application logic, yet editor mark derivation, review overlay state, undo confirmation, and the `canEdit` read-only gating added here are un-tested. Consider adding tests that assert these UI behaviors—such as verifying review marks are derived from receipts, undo triggers the expected mutation, and read-only mode is enforced when `canEdit` is false.</violation>
</file>

<file name="src/features/workspaces/content/workspace-content-reader.ts">

<violation number="1" location="src/features/workspaces/content/workspace-content-reader.ts:50">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The `readBudgetExhausted` short-circuit changes the loop so that once the byte budget is exceeded, every later document/file resolution is skipped with `read_budget_exceeded` without ever calling `readWorkspaceItem`. The nearby comment `// Reads stay ordered so each body is consumed before the shared byte budget advances.` is now stale — after the first budget failure, bodies are no longer consumed at all. Additionally, this is a meaningful behavior-change PR where the new skip-after-exhaustion path is not regression-tested: the existing batch-bounds test uses the same oversized content for every request, so it would pass with both the old and new code. A test mixing a budget-exceeding item with smaller follow-up items would be needed to catch a regression.</violation>

<violation number="2" location="src/features/workspaces/content/workspace-content-reader.ts:107">
P3: The relations RPC call is no longer awaited in the read loop; it is now fired eagerly and only consumed later by `attachRelationPaths` via `Promise.all`. If the loop exits abnormally before `attachRelationPaths` runs—for example a read throws a non-`WorkspacePageSelectionError` and is rethrown by the `catch`, or the `resolutions`/`requests` mismatch `throw` fires—the already-started `listItemRelations` promises are left without a consumer. When one of those asynchronous kernel/network calls later rejects, it becomes an unhandled rejection that is not attributable to the propagated error. Previously the `await` happened inside the loop's try block, so a rejection surfaced deterministically through that same error path. Consider tracking the pending relations so any collected-but-unprocessed promise is cancelled or observed if the loop bails out.</violation>
</file>

<file name="src/features/workspaces/components/WorkspaceLayout.tsx">

<violation number="1" location="src/features/workspaces/components/WorkspaceLayout.tsx:309">
P2: A review opened in one workspace can remain active when the reused shell switches to another workspace, leaving stale review state available to the new document UI and potentially reopening the old review when returning. Key this provider by `workspace.id` (or reset its state whenever `workspaceId` changes) so review state cannot cross workspace lifetimes.</violation>
</file>

<file name="src/features/workspaces/components/ai-chat/AiChatMessageRow.tsx">

<violation number="1" location="src/features/workspaces/components/ai-chat/AiChatMessageRow.tsx:64">
P2: Legacy Code Mode turns can omit the Review/Undo receipt even when a sibling `workspace_edit_item` successfully changed a document: `display.parts` has already folded that tool into a child without edit metadata before this extraction runs. Preserving the edit metadata in the legacy child or extracting the receipt before grouping would keep those edits reviewable.</violation>
</file>

<file name="src/features/workspaces/documents/document-ai-edits.test.ts">

<violation number="1" location="src/features/workspaces/documents/document-ai-edits.test.ts:30">
P3: Coverage gap: the delete path is only tested with a missing ref (ref_not_found) and never with an existing block, and insert_before is never exercised, even though both are core edit ops besides insert_after/replace/replace_all. A test asserting a valid delete actually removes the block (and confirms the editor-child invariant still holds) would guard the most user-visible destructive operation of this AI editing feature.</violation>
</file>

<file name="src/features/workspaces/documents/use-document-edit-review-overlay.ts">

<violation number="1" location="src/features/workspaces/documents/use-document-edit-review-overlay.ts:48">
P2: Reopening the same receipt can briefly show an outdated diff before the server’s fresh verdict arrives. Waiting for the refetch to finish, or removing/invalidation of this key on close, would prevent a changed document from being presented as still reviewable.</violation>

<violation number="2" location="src/features/workspaces/documents/use-document-edit-review-overlay.ts:49">
P2: While the review request is pending, the toolbar already says Reviewing changes but the editor remains editable. A keystroke can change the document before the verdict arrives and make the review or subsequent undo fail; enter read-only mode as soon as the target becomes active and restore it on every exit path.</violation>
</file>

<file name="src/features/workspaces/operations/workspace-tool-schemas.ts">

<violation number="1" location="src/features/workspaces/operations/workspace-tool-schemas.ts:336">
P2: The create output contract now accepts `file`, `flashcard`, and `quiz` items even though this operation can create only `folder` and `document` items. Keep this output type restricted to the two creatable kinds so an invalid create result cannot validate as successful.</violation>

<violation number="2" location="src/features/workspaces/operations/workspace-tool-schemas.ts:364">
P2: An edit result with `itemId: ""` passes output validation but cannot produce a document-edit receipt because downstream receipt construction requires a truthy ID. Constrain the optional field to a non-empty string.</violation>
</file>

<file name="src/features/workspaces/ai/ai-thread-orchestration-contract.ts">

<violation number="1" location="src/features/workspaces/ai/ai-thread-orchestration-contract.ts:252">
P2: The new `getDocumentEditAction` duplicates the document-edit extraction logic that already lives in `ai-chat-document-edit-actions.ts` (`getAiChatDocumentEditGroups`/`readLineChanges`). Both independently decide whether a tool result is a valid edit (`itemId`, `path`, `applied > 0`, `receiptId`) and both parse `lineChanges` into `{added, removed}` for the app's review receipts. Because each path now has its own source of truth, a change to one (for example altering what counts as an eligible edit or how line counts are read) can silently drift from the other. Consider extracting a shared helper that both the codemode normalization path and the direct tool-UI path use, so the two review channels stay consistent.</violation>
</file>

<file name="src/features/workspaces/documents/document-session.ts">

<violation number="1" location="src/features/workspaces/documents/document-session.ts:261">
P2: A human/collaborator edit that lands during applyEdits can be silently lost. The live-document guard (`getCurrentTiptapDocument() !== beforeDocumentText`) runs before an `await this.ctx.storage.get(...)` for the receipt index; anything written to the Yjs doc while that await is in flight slips past the guard, and the later `reconcileCurrentDocument(editResult.document)` overwrites the whole fragment with the snapshot-based AI result, which is then persisted to the kernel. Re-run the content_changed check immediately before (or re-verify `currentHash === afterHash` after) reconciling, so concurrent changes are detected rather than clobbered.</violation>
</file>

<file name="src/features/workspaces/documents/document-ai-edits.ts">

<violation number="1" location="src/features/workspaces/documents/document-ai-edits.ts:175">
P2: Atom line keys currently include generated `aiRef` data, so unchanged atom blocks can appear as one removal plus one addition after ref regeneration. Excluding `aiRef` from the key keeps the `added`/`removed` totals accurate.</violation>
</file>

<file name="src/features/workspaces/documents/tiptap-schema.ts">

<violation number="1" location="src/features/workspaces/documents/tiptap-schema.ts:78">
P2: Enabling `openOnClick` in the shared editor schema makes link clicks navigate while editing, which interferes with cursor placement and text selection in editable documents. Keeping this disabled for editable surfaces avoids accidental navigation.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

This PR wires up new review/undo behavior (DocumentEditReviewExtension and useDocumentEditReviewOverlay) but no tests in the codebase exercise these additions. The existing document-ai-edits.test.ts covers the core edit-application logic, yet editor mark derivation, review overlay state, undo confirmation, and the canEdit read-only gating added here are un-tested. Consider adding tests that assert these UI behaviors—such as verifying review marks are derived from receipts, undo triggers the expected mutation, and read-only mode is enforced when canEdit is false.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/document-editor/DocumentEditorSurface.tsx, line 139:

<comment>This PR wires up new review/undo behavior (`DocumentEditReviewExtension` and `useDocumentEditReviewOverlay`) but no tests in the codebase exercise these additions. The existing `document-ai-edits.test.ts` covers the core edit-application logic, yet editor mark derivation, review overlay state, undo confirmation, and the `canEdit` read-only gating added here are un-tested. Consider adding tests that assert these UI behaviors—such as verifying review marks are derived from receipts, undo triggers the expected mutation, and read-only mode is enforced when `canEdit` is false.</comment>

<file context>
@@ -123,6 +136,7 @@ function getDocumentEditorExtensions(collaborationSession: DocumentCollaboration
 
 	return [
 		...baseExtensions,
+		DocumentEditReviewExtension,
 		Collaboration.configure({
 			document: collaborationSession.ydoc,
</file context>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

The readBudgetExhausted short-circuit changes the loop so that once the byte budget is exceeded, every later document/file resolution is skipped with read_budget_exceeded without ever calling readWorkspaceItem. The nearby comment // Reads stay ordered so each body is consumed before the shared byte budget advances. is now stale — after the first budget failure, bodies are no longer consumed at all. Additionally, this is a meaningful behavior-change PR where the new skip-after-exhaustion path is not regression-tested: the existing batch-bounds test uses the same oversized content for every request, so it would pass with both the old and new code. A test mixing a budget-exceeding item with smaller follow-up items would be needed to catch a regression.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/content/workspace-content-reader.ts, line 50:

<comment>The `readBudgetExhausted` short-circuit changes the loop so that once the byte budget is exceeded, every later document/file resolution is skipped with `read_budget_exceeded` without ever calling `readWorkspaceItem`. The nearby comment `// Reads stay ordered so each body is consumed before the shared byte budget advances.` is now stale — after the first budget failure, bodies are no longer consumed at all. Additionally, this is a meaningful behavior-change PR where the new skip-after-exhaustion path is not regression-tested: the existing batch-bounds test uses the same oversized content for every request, so it would pass with both the old and new code. A test mixing a budget-exceeding item with smaller follow-up items would be needed to catch a regression.</comment>

<file context>
@@ -49,6 +47,7 @@ export async function readWorkspaceContent(input: {
 	const results: WorkspaceContentReadResult[] = [];
 	const readyResults: PendingReadyResult[] = [];
 	let returnedContentBytes = 0;
+	let readBudgetExhausted = false;
 
 	// Reads stay ordered so each body is consumed before the shared byte budget advances.
</file context>

[cubic-dev-ai]

P2: A review opened in one workspace can remain active when the reused shell switches to another workspace, leaving stale review state available to the new document UI and potentially reopening the old review when returning. Key this provider by workspace.id (or reset its state whenever workspaceId changes) so review state cannot cross workspace lifetimes.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/WorkspaceLayout.tsx, line 309:

<comment>A review opened in one workspace can remain active when the reused shell switches to another workspace, leaving stale review state available to the new document UI and potentially reopening the old review when returning. Key this provider by `workspace.id` (or reset its state whenever `workspaceId` changes) so review state cannot cross workspace lifetimes.</comment>

<file context>
@@ -305,11 +306,13 @@ export function WorkspaceShell({
-				) : (
-					workspaceInteractionContent
-				)}
+				<DocumentEditReviewProvider workspaceId={workspace.id}>
+					{hasHeavyViewerRuntimeItems ? (
+						<WorkspacePdfEngineProvider>{workspaceInteractionContent}</WorkspacePdfEngineProvider>
</file context>

[cubic-dev-ai]

P2: Legacy Code Mode turns can omit the Review/Undo receipt even when a sibling workspace_edit_item successfully changed a document: display.parts has already folded that tool into a child without edit metadata before this extraction runs. Preserving the edit metadata in the legacy child or extracting the receipt before grouping would keep those edits reviewable.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/ai-chat/AiChatMessageRow.tsx, line 64:

<comment>Legacy Code Mode turns can omit the Review/Undo receipt even when a sibling `workspace_edit_item` successfully changed a document: `display.parts` has already folded that tool into a child without edit metadata before this extraction runs. Preserving the edit metadata in the legacy child or extracting the receipt before grouping would keep those edits reviewable.</comment>

<file context>
@@ -57,6 +59,10 @@ export default function AiChatMessageRow({
 	const copyableText = !isStreaming ? getCopyableMessageText(message) : "";
+	const documentEditGroups =
+		isAssistant && display?.kind === "content" && !isStreaming
+			? getAiChatDocumentEditGroups(display.parts)
+			: [];
 
</file context>

[cubic-dev-ai]

P2: While the review request is pending, the toolbar already says Reviewing changes but the editor remains editable. A keystroke can change the document before the verdict arrives and make the review or subsequent undo fail; enter read-only mode as soon as the target becomes active and restore it on every exit path.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/use-document-edit-review-overlay.ts, line 49:

<comment>While the review request is pending, the toolbar already says Reviewing changes but the editor remains editable. A keystroke can change the document before the verdict arrives and make the review or subsequent undo fail; enter read-only mode as soon as the target becomes active and restore it on every exit path.</comment>

<file context>
@@ -0,0 +1,79 @@
+		}
+
+		const review = reviewQuery.data;
+		if (!review) {
+			return;
+		}
</file context>

[cubic-dev-ai]

P2: A human/collaborator edit that lands during applyEdits can be silently lost. The live-document guard (getCurrentTiptapDocument() !== beforeDocumentText) runs before an await this.ctx.storage.get(...) for the receipt index; anything written to the Yjs doc while that await is in flight slips past the guard, and the later reconcileCurrentDocument(editResult.document) overwrites the whole fragment with the snapshot-based AI result, which is then persisted to the kernel. Re-run the content_changed check immediately before (or re-verify currentHash === afterHash after) reconciling, so concurrent changes are detected rather than clobbered.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/document-session.ts, line 261:

<comment>A human/collaborator edit that lands during applyEdits can be silently lost. The live-document guard (`getCurrentTiptapDocument() !== beforeDocumentText`) runs before an `await this.ctx.storage.get(...)` for the receipt index; anything written to the Yjs doc while that await is in flight slips past the guard, and the later `reconcileCurrentDocument(editResult.document)` overwrites the whole fragment with the snapshot-based AI result, which is then persisted to the kernel. Re-run the content_changed check immediately before (or re-verify `currentHash === afterHash` after) reconciling, so concurrent changes are detected rather than clobbered.</comment>

<file context>
@@ -147,81 +175,174 @@ export class DocumentSession extends YServer {
+		);
 
-		await this.persistYDoc();
+		this.reconcileCurrentDocument(editResult.document);
+		const persistedUpdate = Y.encodeStateAsUpdate(this.document);
+		await this.ctx.storage.transaction(async (transaction) => {
</file context>

[cubic-dev-ai]

P2: Atom line keys currently include generated aiRef data, so unchanged atom blocks can appear as one removal plus one addition after ref regeneration. Excluding aiRef from the key keeps the added/removed totals accurate.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/document-ai-edits.ts, line 175:

<comment>Atom line keys currently include generated `aiRef` data, so unchanged atom blocks can appear as one removal plus one addition after ref regeneration. Excluding `aiRef` from the key keeps the `added`/`removed` totals accurate.</comment>

<file context>
@@ -0,0 +1,302 @@
+			}
+			// A rule or a formula holds no text but still occupies a line.
+			if (node.isAtom) {
+				countLine(JSON.stringify(node.toJSON()));
+				return false;
+			}
</file context>

[cubic-dev-ai]

P2: Enabling openOnClick in the shared editor schema makes link clicks navigate while editing, which interferes with cursor placement and text selection in editable documents. Keeping this disabled for editable surfaces avoids accidental navigation.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/tiptap-schema.ts, line 78:

<comment>Enabling `openOnClick` in the shared editor schema makes link clicks navigate while editing, which interferes with cursor placement and text selection in editable documents. Keeping this disabled for editable surfaces avoids accidental navigation.</comment>

<file context>
@@ -40,7 +75,7 @@ export function getTiptapDocumentSchemaExtensions({
 		Highlight,
 		Link.configure({
-			openOnClick: false,
+			openOnClick: true,
 			autolink: true,
 			defaultProtocol: "https",
</file context>

Suggested change

	openOnClick: true,
	openOnClick: false,

[cubic-dev-ai]

P3: Coverage gap: the delete path is only tested with a missing ref (ref_not_found) and never with an existing block, and insert_before is never exercised, even though both are core edit ops besides insert_after/replace/replace_all. A test asserting a valid delete actually removes the block (and confirms the editor-child invariant still holds) would guard the most user-visible destructive operation of this AI editing feature.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/document-ai-edits.test.ts, line 30:

<comment>Coverage gap: the delete path is only tested with a missing ref (ref_not_found) and never with an existing block, and insert_before is never exercised, even though both are core edit ops besides insert_after/replace/replace_all. A test asserting a valid delete actually removes the block (and confirms the editor-child invariant still holds) would guard the most user-visible destructive operation of this AI editing feature.</comment>

<file context>
@@ -0,0 +1,109 @@
+		const firstRef = await getRef(document, "p");
+		const result = await applyDocumentAiEdits(document, [
+			{ html: "<p>Updated</p>", op: "replace", ref: firstRef },
+			{ op: "delete", ref: "b_missingref00.r_0000000000" },
+		]);
+
</file context>

[cubic-dev-ai]

P3: The relations RPC call is no longer awaited in the read loop; it is now fired eagerly and only consumed later by attachRelationPaths via Promise.all. If the loop exits abnormally before attachRelationPaths runs—for example a read throws a non-WorkspacePageSelectionError and is rethrown by the catch, or the resolutions/requests mismatch throw fires—the already-started listItemRelations promises are left without a consumer. When one of those asynchronous kernel/network calls later rejects, it becomes an unhandled rejection that is not attributable to the propagated error. Previously the await happened inside the loop's try block, so a rejection surfaced deterministically through that same error path. Consider tracking the pending relations so any collected-but-unprocessed promise is cancelled or observed if the loop bails out.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/content/workspace-content-reader.ts, line 107:

<comment>The relations RPC call is no longer awaited in the read loop; it is now fired eagerly and only consumed later by `attachRelationPaths` via `Promise.all`. If the loop exits abnormally before `attachRelationPaths` runs—for example a read throws a non-`WorkspacePageSelectionError` and is rethrown by the `catch`, or the `resolutions`/`requests` mismatch `throw` fires—the already-started `listItemRelations` promises are left without a consumer. When one of those asynchronous kernel/network calls later rejects, it becomes an unhandled rejection that is not attributable to the propagated error. Previously the `await` happened inside the loop's try block, so a rejection surfaced deterministically through that same error path. Consider tracking the pending relations so any collected-but-unprocessed promise is cancelled or observed if the loop bails out.</comment>

<file context>
@@ -86,20 +95,16 @@ export async function readWorkspaceContent(input: {
 				item: resolution.item,
 				read,
-				relations: await input.kernel.listItemRelations({ itemId: resolution.item.id }),
+				relations: input.kernel.listItemRelations({ itemId: resolution.item.id }),
 			};
 			readyResults.push(pending);
</file context>

[cubic-dev-ai]

13 issues found and verified against the latest diff

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/features/workspaces/documents/use-document-collaboration-session.ts">

<violation number="1" location="src/features/workspaces/documents/use-document-collaboration-session.ts:209">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The new readiness gate defers `markReady()` when a previously synced session has an empty tiptap Yjs fragment. This is a behavior change for empty or newly created documents, yet there is no test exercising this branch. Adding a regression test—for example, mocking the persistence layer and asserting that `markReady` is skipped when the fragment length is zero—would guard against future accidental changes to this gating logic.</violation>
</file>

<file name="src/features/workspaces/operations/workspace-tool-schemas.ts">

<violation number="1" location="src/features/workspaces/operations/workspace-tool-schemas.ts:294">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The only targeted-edit example in `workspaceEditItemInputExamples` contains a fabricated `ref` value that can never match a real document block. Since the tool description tells the model to "copy data-ref into the 'ref' field," an AI may emit this hard-coded ref verbatim, causing every targeted edit to fail with `ref_not_found` or `stale_target`. Consider replacing the literal ref with a clear placeholder or an inline comment indicating that the value must come from the document read.</violation>
</file>

<file name="src/features/workspaces/components/document-editor/document-editor-toolbar-actions.tsx">

<violation number="1" location="src/features/workspaces/components/document-editor/document-editor-toolbar-actions.tsx:79">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

This PR switches toolbar actions from font-size semantics to text-style semantics and adds a new Heading 4 action, but no tests cover the updated `documentTextStyleActions`, the new `heading4` item, or the refactored `getTextStyleIcon` / `isTextStyle` helpers. For a behavior-change PR like this, regression-style tests asserting that each action maps to the correct editor command and that active-state detection works would be practical and valuable. Consider adding unit or integration tests for the toolbar action definitions and their icon/active-state mappings.</violation>
</file>

<file name="src/features/workspaces/ai/ai-thread-orchestration-contract.ts">

<violation number="1" location="src/features/workspaces/ai/ai-thread-orchestration-contract.ts:252">
P1: Custom agent: **Flag AI Slop and Fabricated Changes**

New behavior-changing logic (`getDocumentEditAction`, `withoutCallActions`, `stripAIThreadToolUiMetadata`) is not covered by unit tests. The PR should include tests asserting the various branches (e.g., method/state gating, path fallback, lineChanges inclusion, metadata stripping) to prevent regressions.</violation>
</file>

<file name="src/features/workspaces/components/ai-chat/ai-chat-document-edit-actions.ts">

<violation number="1" location="src/features/workspaces/components/ai-chat/ai-chat-document-edit-actions.ts:20">
P1: Custom agent: **Flag AI Slop and Fabricated Changes**

The new `getAiChatDocumentEditGroups` / `addToGroup` receipt-grouping logic is a core part of this behavior-change PR (grouping document edit receipts and tallying line changes per item), yet it has no tests. The rule flags behavior-change PRs where changed behavior is not exercised by tests, especially when regression-style assertions are practical. Consider adding unit tests that cover: deduplication by `receiptId`, aggregation of `lineChanges` across multiple receipts, filtering of `applied === 0` results, default handling when `lineChanges` is missing, and interaction between `toolGroupPart` children and standalone tool UI parts.</violation>
</file>

<file name="src/features/workspaces/components/ai-chat/ai-chat-codemode-activity.ts">

<violation number="1" location="src/features/workspaces/components/ai-chat/ai-chat-codemode-activity.ts:16">
P2: Document receipt line counts are missing from the declared document-edit action contract even though normalized actions carry `lineChanges` and the receipt grouping consumes it for `Lines: +N −M`. Add the optional numeric `lineChanges` field to the action type and validate it in `isDocumentEditAction`; relying on an untyped runtime extra leaves consumers unable to safely use the tally and lets malformed counts pass the guard.</violation>
</file>

<file name="src/features/workspaces/documents/document-session.ts">

<violation number="1" location="src/features/workspaces/documents/document-session.ts:257">
P2: A turn that edits one document more than eight times loses the first receipt IDs before its chat row is opened, so the row's review and undo requests return `not_found` instead of showing that turn's changes. Retain all receipt IDs referenced by the active turn, or have the UI submit only a retained latest chain.</violation>

<violation number="2" location="src/features/workspaces/documents/document-session.ts:442">
P1: Reconciling an AI edit (or undo) now injects the document into the Yjs fragment without first clearing existing content, and without the transaction wrapper the old `replaceCurrentDocument` had. Since `prosemirrorJSONToYXmlFragment` inserts rather than replaces (which is why the original code called `fragment.delete(0, fragment.length)` first), each AI edit/undo/ref-hydration run can leave a duplicate copy of the whole document appended to what is already there — corrupting the persisted YDoc and the receipt `afterHash`/`beforeHash` bookkeeping. Please clear the fragment (and keep the `document.transact(...)` boundary with origin) before writing, as the previous implementation did. If the y-tiptap version used does clear on its own, please confirm that explicitly, since it is load-bearing here.</violation>
</file>

<file name="src/features/workspaces/components/WorkspaceLayout.tsx">

<violation number="1" location="src/features/workspaces/components/WorkspaceLayout.tsx:309">
P2: Switching between cached workspaces can retain the previous workspace's active document review, so returning to that workspace may reopen an old AI diff unexpectedly. Key the provider by `workspace.id` or reset `activeReview` when the workspace changes.</violation>
</file>

<file name="src/features/workspaces/documents/document-edit-review-queries.ts">

<violation number="1" location="src/features/workspaces/documents/document-edit-review-queries.ts:24">
P2: Reopening a receipt can show an obsolete AI diff and Undo action because `staleTime: 0` triggers a background refetch but does not disable React Query caching. Removing the inactive cache and/or clearing or gating review data while the verdict refetches would preserve the server-side staleness guarantee.</violation>
</file>

<file name="src/features/workspaces/ai/ai-thread-tool-ui-metadata.ts">

<violation number="1" location="src/features/workspaces/ai/ai-thread-tool-ui-metadata.ts:45">
P2: The receipt UUID is still exposed to the model: Code Mode can copy `documentEditReceiptId` into an ordinary result field before this key-based scrub runs. Keep the receipt in an app-side execution record rather than returning it through the model-visible connector result, or use a non-model side channel for the action metadata.</violation>
</file>

<file name="src/features/workspaces/documents/document-edit-review-extension.ts">

<violation number="1" location="src/features/workspaces/documents/document-edit-review-extension.ts:167">
P2: Nested block atoms can be changed without any visible review highlight, making part of an AI edit appear unchanged. Traversing only top-level children misses atoms inside blockquote, table, and list content; traversing descendants for atom decorations would cover these structural edits.</violation>
</file>

<file name="src/features/workspaces/content/workspace-content-reader.ts">

<violation number="1" location="src/features/workspaces/content/workspace-content-reader.ts:80">
P3: Once any single item exceeds the read byte budget, the new `readBudgetExhausted` flag short-circuits every remaining item to `read_budget_exceeded`, even ones that would fit within the remaining budget. Because over-budget items never advance `returnedContentBytes`, a later small document that previously would have been returned is now silently dropped, so a mixed-size batch yields less context to the model than before. This looks like an intentional optimization (avoiding re-reading after the budget is blown), but please confirm the reduced result set is acceptable; if not, only the over-budget item should be failed while later in-budget items keep reading.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P1: Custom agent: Flag AI Slop and Fabricated Changes

New behavior-changing logic (getDocumentEditAction, withoutCallActions, stripAIThreadToolUiMetadata) is not covered by unit tests. The PR should include tests asserting the various branches (e.g., method/state gating, path fallback, lineChanges inclusion, metadata stripping) to prevent regressions.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/ai/ai-thread-orchestration-contract.ts, line 252:

<comment>New behavior-changing logic (`getDocumentEditAction`, `withoutCallActions`, `stripAIThreadToolUiMetadata`) is not covered by unit tests. The PR should include tests asserting the various branches (e.g., method/state gating, path fallback, lineChanges inclusion, metadata stripping) to prevent regressions.</comment>

<file context>
@@ -219,6 +249,37 @@ function normalizeCall(call: z.output<typeof rawOrchestrationCallSchema>) {
 	};
 }
 
+function getDocumentEditAction(call: z.output<typeof rawOrchestrationCallSchema>) {
+	if (call.method !== "workspace_edit_item" || call.state !== "applied") {
+		return undefined;
</file context>

[cubic-dev-ai]

P1: Custom agent: Flag AI Slop and Fabricated Changes

The new getAiChatDocumentEditGroups / addToGroup receipt-grouping logic is a core part of this behavior-change PR (grouping document edit receipts and tallying line changes per item), yet it has no tests. The rule flags behavior-change PRs where changed behavior is not exercised by tests, especially when regression-style assertions are practical. Consider adding unit tests that cover: deduplication by receiptId, aggregation of lineChanges across multiple receipts, filtering of applied === 0 results, default handling when lineChanges is missing, and interaction between toolGroupPart children and standalone tool UI parts.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/ai-chat/ai-chat-document-edit-actions.ts, line 20:

<comment>The new `getAiChatDocumentEditGroups` / `addToGroup` receipt-grouping logic is a core part of this behavior-change PR (grouping document edit receipts and tallying line changes per item), yet it has no tests. The rule flags behavior-change PRs where changed behavior is not exercised by tests, especially when regression-style assertions are practical. Consider adding unit tests that cover: deduplication by `receiptId`, aggregation of `lineChanges` across multiple receipts, filtering of `applied === 0` results, default handling when `lineChanges` is missing, and interaction between `toolGroupPart` children and standalone tool UI parts.</comment>

<file context>
@@ -0,0 +1,100 @@
+	receiptIds: string[];
+}
+
+export function getAiChatDocumentEditGroups(
+	parts: readonly AiChatRenderablePart[],
+): AiChatDocumentEditGroup[] {
</file context>

[cubic-dev-ai]

P1: Reconciling an AI edit (or undo) now injects the document into the Yjs fragment without first clearing existing content, and without the transaction wrapper the old replaceCurrentDocument had. Since prosemirrorJSONToYXmlFragment inserts rather than replaces (which is why the original code called fragment.delete(0, fragment.length) first), each AI edit/undo/ref-hydration run can leave a duplicate copy of the whole document appended to what is already there — corrupting the persisted YDoc and the receipt afterHash/beforeHash bookkeeping. Please clear the fragment (and keep the document.transact(...) boundary with origin) before writing, as the previous implementation did. If the y-tiptap version used does clear on its own, please confirm that explicitly, since it is load-bearing here.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/document-session.ts, line 441:

<comment>Reconciling an AI edit (or undo) now injects the document into the Yjs fragment without first clearing existing content, and without the transaction wrapper the old `replaceCurrentDocument` had. Since `prosemirrorJSONToYXmlFragment` inserts rather than replaces (which is why the original code called `fragment.delete(0, fragment.length)` first), each AI edit/undo/ref-hydration run can leave a duplicate copy of the whole document appended to what is already there — corrupting the persisted YDoc and the receipt `afterHash`/`beforeHash` bookkeeping. Please clear the fragment (and keep the `document.transact(...)` boundary with origin) before writing, as the previous implementation did. If the y-tiptap version used does clear on its own, please confirm that explicitly, since it is load-bearing here.</comment>

<file context>
@@ -230,32 +351,94 @@ export class DocumentSession extends YServer {
-		}, this);
+	private reconcileCurrentDocument(document: TiptapDocumentJson) {
+		const fragment = this.document.getXmlFragment(tiptapDocumentYjsField);
+		prosemirrorJSONToYXmlFragment(getTiptapDocumentSchema(), document, fragment);
 	}
 
</file context>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

The new readiness gate defers markReady() when a previously synced session has an empty tiptap Yjs fragment. This is a behavior change for empty or newly created documents, yet there is no test exercising this branch. Adding a regression test—for example, mocking the persistence layer and asserting that markReady is skipped when the fragment length is zero—would guard against future accidental changes to this gating logic.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/use-document-collaboration-session.ts, line 209:

<comment>The new readiness gate defers `markReady()` when a previously synced session has an empty tiptap Yjs fragment. This is a behavior change for empty or newly created documents, yet there is no test exercising this branch. Adding a regression test—for example, mocking the persistence layer and asserting that `markReady` is skipped when the fragment length is zero—would guard against future accidental changes to this gating logic.</comment>

<file context>
@@ -203,7 +204,10 @@ function createActiveDocumentSession(input: {
-			if (wasServerSynced === localDocumentReadyValue) {
+			if (
+				wasServerSynced === localDocumentReadyValue &&
+				session.ydoc.getXmlFragment(tiptapDocumentYjsField).length > 0
+			) {
 				markReady();
</file context>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

The only targeted-edit example in workspaceEditItemInputExamples contains a fabricated ref value that can never match a real document block. Since the tool description tells the model to "copy data-ref into the 'ref' field," an AI may emit this hard-coded ref verbatim, causing every targeted edit to fail with ref_not_found or stale_target. Consider replacing the literal ref with a clear placeholder or an inline comment indicating that the value must come from the document read.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/operations/workspace-tool-schemas.ts, line 294:

<comment>The only targeted-edit example in `workspaceEditItemInputExamples` contains a fabricated `ref` value that can never match a real document block. Since the tool description tells the model to "copy data-ref into the 'ref' field," an AI may emit this hard-coded ref verbatim, causing every targeted edit to fail with `ref_not_found` or `stale_target`. Consider replacing the literal ref with a clear placeholder or an inline comment indicating that the value must come from the document read.</comment>

<file context>
@@ -282,15 +285,27 @@ export const workspaceDeleteItemsInputExamples = createInputExamples<
+		edits: [
+			{
+				op: "replace",
+				ref: "b_JQrkL4Neurv2.r_6sNqkQxDdy",
+				html: "<p>Updated paragraph.</p>",
+			},
</file context>

Suggested change

	ref: "b_JQrkL4Neurv2.r_6sNqkQxDdy",
	// ref comes from a data-ref attribute in the document's HTML read
	ref: "<data-ref from the read document>",

[cubic-dev-ai]

P2: Switching between cached workspaces can retain the previous workspace's active document review, so returning to that workspace may reopen an old AI diff unexpectedly. Key the provider by workspace.id or reset activeReview when the workspace changes.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/WorkspaceLayout.tsx, line 309:

<comment>Switching between cached workspaces can retain the previous workspace's active document review, so returning to that workspace may reopen an old AI diff unexpectedly. Key the provider by `workspace.id` or reset `activeReview` when the workspace changes.</comment>

<file context>
@@ -305,11 +306,13 @@ export function WorkspaceShell({
-				) : (
-					workspaceInteractionContent
-				)}
+				<DocumentEditReviewProvider workspaceId={workspace.id}>
+					{hasHeavyViewerRuntimeItems ? (
+						<WorkspacePdfEngineProvider>{workspaceInteractionContent}</WorkspacePdfEngineProvider>
</file context>

[cubic-dev-ai]

P2: Reopening a receipt can show an obsolete AI diff and Undo action because staleTime: 0 triggers a background refetch but does not disable React Query caching. Removing the inactive cache and/or clearing or gating review data while the verdict refetches would preserve the server-side staleness guarantee.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/document-edit-review-queries.ts, line 24:

<comment>Reopening a receipt can show an obsolete AI diff and Undo action because `staleTime: 0` triggers a background refetch but does not disable React Query caching. Removing the inactive cache and/or clearing or gating review data while the verdict refetches would preserve the server-side staleness guarantee.</comment>

<file context>
@@ -0,0 +1,26 @@
+		// whether the document still matches the receipt, and a verdict cached
+		// from an earlier open would mark the reader's own later writing as the
+		// assistant's.
+		staleTime: 0,
+	});
+}
</file context>

[cubic-dev-ai]

P2: The receipt UUID is still exposed to the model: Code Mode can copy documentEditReceiptId into an ordinary result field before this key-based scrub runs. Keep the receipt in an app-side execution record rather than returning it through the model-visible connector result, or use a non-model side channel for the action metadata.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/ai/ai-thread-tool-ui-metadata.ts, line 45:

<comment>The receipt UUID is still exposed to the model: Code Mode can copy `documentEditReceiptId` into an ordinary result field before this key-based scrub runs. Keep the receipt in an app-side execution record rather than returning it through the model-visible connector result, or use a non-model side channel for the action metadata.</comment>

<file context>
@@ -0,0 +1,52 @@
+
+	return Object.fromEntries(
+		Object.entries(value).flatMap(([key, entry]) =>
+			key === aiThreadToolUiMetadataKey ? [] : [[key, stripAIThreadToolUiMetadata(entry)]],
+		),
+	);
</file context>

[cubic-dev-ai]

P2: Nested block atoms can be changed without any visible review highlight, making part of an AI edit appear unchanged. Traversing only top-level children misses atoms inside blockquote, table, and list content; traversing descendants for atom decorations would cover these structural edits.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/document-edit-review-extension.ts, line 167:

<comment>Nested block atoms can be changed without any visible review highlight, making part of an AI edit appear unchanged. Traversing only top-level children misses atoms inside blockquote, table, and list content; traversing descendants for atom decorations would cover these structural edits.</comment>

<file context>
@@ -0,0 +1,204 @@
+	from: number,
+	to: number,
+) {
+	document.forEach((node, offset) => {
+		const end = offset + node.nodeSize;
+		if (end <= from || offset >= to || !node.isAtom) {
</file context>

[cubic-dev-ai]

P3: Once any single item exceeds the read byte budget, the new readBudgetExhausted flag short-circuits every remaining item to read_budget_exceeded, even ones that would fit within the remaining budget. Because over-budget items never advance returnedContentBytes, a later small document that previously would have been returned is now silently dropped, so a mixed-size batch yields less context to the model than before. This looks like an intentional optimization (avoiding re-reading after the budget is blown), but please confirm the reduced result set is acceptable; if not, only the over-budget item should be failed while later in-budget items keep reading.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/content/workspace-content-reader.ts, line 80:

<comment>Once any single item exceeds the read byte budget, the new `readBudgetExhausted` flag short-circuits every remaining item to `read_budget_exceeded`, even ones that would fit within the remaining budget. Because over-budget items never advance `returnedContentBytes`, a later small document that previously would have been returned is now silently dropped, so a mixed-size batch yields less context to the model than before. This looks like an intentional optimization (avoiding re-reading after the budget is blown), but please confirm the reduced result set is acceptable; if not, only the over-budget item should be failed while later in-budget items keep reading.</comment>

<file context>
@@ -72,6 +71,16 @@ export async function readWorkspaceContent(input: {
+			status: "failed" as const,
+			...(resolution.item.type === "file" ? { type: "file" as const } : {}),
+		};
+		if (readBudgetExhausted) {
+			results.push(readBudgetFailure);
+			continue;
</file context>

[cubic-dev-ai]

16 issues found and verified against the latest diff

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/features/workspaces/components/WorkspaceLayout.tsx">

<violation number="1" location="src/features/workspaces/components/WorkspaceLayout.tsx:309">
P2: Navigating between workspace routes can retain the previous workspace's active review and, for a matching item ID, show its diff or submit its receipt IDs under the new workspace. Key the provider by `workspace.id` (or reset its review state when the workspace changes) to keep review state scoped to one workspace.</violation>
</file>

<file name="src/features/workspaces/components/ai-chat/ai-chat-codemode-activity.ts">

<violation number="1" location="src/features/workspaces/components/ai-chat/ai-chat-codemode-activity.ts:93">
P2: Document-edit receipts can show corrupted line counts when a compact action carries malformed `lineChanges`: this guard accepts the action, while the grouping code later sums those values directly. Including the optional `lineChanges` shape in the action contract and validating both fields as numbers would keep malformed activities out of the receipt flow.</violation>
</file>

<file name="src/features/workspaces/components/ai-chat/AiChatDocumentEditActions.tsx">

<violation number="1" location="src/features/workspaces/components/ai-chat/AiChatDocumentEditActions.tsx:43">
P2: Receipt rows do not show the required persistent “Edited since” or “Undone” state, so the transcript gives no indication when a document has diverged from or already reverted the AI edit. Adding a live row status (or updating the group data to provide one) would preserve that feedback without requiring the user to open each row.</violation>
</file>

<file name="src/features/workspaces/documents/use-document-edit-receipt-undo.ts">

<violation number="1" location="src/features/workspaces/documents/use-document-edit-receipt-undo.ts:22">
P2: A pending undo can close a different document's review when it finishes. `onSuccess` unconditionally clears the shared review state, so it should hide the review only when the active item and receipt IDs still match this mutation's target.</violation>
</file>

<file name="src/features/workspaces/documents/document-html-chunk.ts">

<violation number="1" location="src/features/workspaces/documents/document-html-chunk.ts:44">
P2: A document containing one very large top-level block cannot be read: the first chunk is allowed to exceed the advertised bound, then the workspace reader rejects that result when it exceeds its 2 MiB response budget. The chunking contract needs an explicit oversized-block policy (for example, a dedicated failure/truncation path or a coordinated larger budget) rather than emitting an unbounded chunk.</violation>
</file>

<file name="src/features/workspaces/documents/document-ai-edits.ts">

<violation number="1" location="src/features/workspaces/documents/document-ai-edits.ts:181">
P2: Receipts report unchanged horizontal rules and math blocks as removed and re-added after a `replace_all`, inflating the user-visible Lines count. Fingerprint atom content after clearing the generated AI ref, as the visible-content comparison already does.</violation>
</file>

<file name="src/features/workspaces/documents/use-document-edit-review-overlay.ts">

<violation number="1" location="src/features/workspaces/documents/use-document-edit-review-overlay.ts:14">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

This new hook introduces substantial UI and editor-state behavior (review query handling, toast errors, toggling editor editability, showing/hiding review decorations), but none of it is exercised by tests in the visible diff. For a behavior-change PR, regression-style tests are practical here—for example, asserting that a ready status calls `setEditable(false)` and `showDocumentEditReview`, or that an error/unavailable status calls `toast.error` and `hideReview()`. Adding tests would prevent silent regressions in the review/undo workflow.</violation>

<violation number="2" location="src/features/workspaces/documents/use-document-edit-review-overlay.ts:49">
P1: The document remains editable while the review verdict is loading, so users can type after opening a receipt and have those edits presented as part of the AI diff. Disable editability as soon as an active review target exists, and restore it through cleanup for loading, error, and unavailable-result paths.</violation>
</file>

<file name="src/features/workspaces/documents/document-edit-review-queries.ts">

<violation number="1" location="src/features/workspaces/documents/document-edit-review-queries.ts:24">
P2: The `staleTime: 0` here is being used to mean "not cached," but staleTime only marks data as stale; React Query still returns the cached response synchronously on reopen (within the default ~5min gcTime) and refetches in the background. The overlay effect renders review marks as soon as `reviewQuery.data` exists, so on reopen there is a window where a stale `ready` verdict from a previous open can mark the reader's own later edits as AI insertions — the exact failure the comment says this avoids. To honor the stated intent, disable caching outright (e.g., `gcTime: 0`) or gate showing the marks until the refetch settles, rather than relying on `staleTime: 0`.</violation>
</file>

<file name="src/features/workspaces/components/document-editor/DocumentToolbar.tsx">

<violation number="1" location="src/features/workspaces/components/document-editor/DocumentToolbar.tsx:55">
P2: There's a transient inconsistency between the toolbar and the actual review state. As soon as a receipt is clicked, activeReview is set synchronously and this toolbar switches to 'Reviewing changes' with an active Undo button, but the review marks are only applied and the editor locked in use-document-edit-review-overlay after the receipt query returns status 'ready'. During that loading window the document is still fully editable and unmarked, and Undo can be clicked before the server has confirmed the receipt is still the latest and matches the live doc — a stale/out-of-order click will surface a 'not_latest'/'content_changed' error and end the review abruptly. Consider gating the review-mode toolbar (and the Undo affordance) on the review having loaded as ready, or locking the editor the moment a review is initiated so the 'reading mode' invariant holds for the whole window.</violation>
</file>

<file name="src/features/workspaces/documents/tiptap-schema.ts">

<violation number="1" location="src/features/workspaces/documents/tiptap-schema.ts:78">
P3: This flips link-click behavior for the entire collaborative document editor, not just the AI-review read-only surface. `openOnClick: true` on the shared schema means a plain click on any link in any document now navigates the user away, which interferes with placing the cursor or selecting text around a link during normal editing (the server-side AI serialization never reads this setting, so it only affects live editing). If the intent was to let links open in the review view, consider scoping that to the review/decoration layer instead of changing the interactive editor for all documents.</violation>
</file>

<file name="src/features/workspaces/content/workspace-content-reader.ts">

<violation number="1" location="src/features/workspaces/content/workspace-content-reader.ts:80">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

Behavior change: after the first item exceeds the batch byte budget, all later items are immediately skipped with `read_budget_exceeded` even if they would have fit. This changes what the AI reader returns for subsequent documents (e.g., a small document after an oversized one now silently fails). The existing batch-budget test uses 20 identical large documents and does not distinguish the old per-item check from the new early-exit flag, so the new skip-after-exhaustion semantics are untested. Consider adding a regression-style assertion that verifies the early-exit behavior, such as an oversized document followed by a small one.</violation>

<violation number="2" location="src/features/workspaces/content/workspace-content-reader.ts:98">
P2: A single over-budget document now causes every later item in the batch to be skipped. Because setting readBudgetExhausted on the first oversized read leaves returnedContentBytes unchanged, the following valid (smaller) documents that would fit within the remaining budget are also rejected with read_budget_exceeded and never read — content the model previously received is now dropped. Consider only flipping readBudgetExhausted when the budget is genuinely exhausted by already-returned bytes, not when one item alone exceeds the cap.</violation>

<violation number="3" location="src/features/workspaces/content/workspace-content-reader.ts:107">
P2: The relations promises are now created but left unhandled until attachRelationPaths runs after the whole loop completes. Because each iteration suspends on `await readWorkspaceItem`, a fast-failing `listItemRelations` call can settle during that window with no rejection handler attached, which can surface as an unhandled rejection and fail the request in workerd. If the intent is to parallelize the relations reads, attach a rejection handler to each promise as it is created (or use Promise.allSettled), or keep the inline await; otherwise resolve errors the same way as before.</violation>
</file>

<file name="src/features/workspaces/model/workspace-ai-context-prompt.ts">

<violation number="1" location="src/features/workspaces/model/workspace-ai-context-prompt.ts:78">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The added `outline.totalItems === 0` branch changes the AI context prompt text for empty workspaces but has no test coverage. A straightforward unit test asserting the exact output of `formatWorkspaceAiContextOutline` for an empty outline is practical and would prevent regressions. Please add a regression test for this branch.</violation>
</file>

<file name="src/features/workspaces/documents/use-document-collaboration-session.ts">

<violation number="1" location="src/features/workspaces/documents/use-document-collaboration-session.ts:207">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

This diff changes when a document collaboration session is marked 'ready' from local IndexedDB persistence: it now additionally requires the tiptap document XML fragment to be non-empty, beyond the existing server-synced marker check. This is a real behavior change that prevents surfacing stale/empty local documents, but it appears to have no test coverage. A regression-style test here is practical — verifying that a persisted ready marker with an empty ydoc fragment does NOT mark the session ready, while a non-empty fragment does, would validate the new guard and protect against future regressions.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P1: The document remains editable while the review verdict is loading, so users can type after opening a receipt and have those edits presented as part of the AI diff. Disable editability as soon as an active review target exists, and restore it through cleanup for loading, error, and unavailable-result paths.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/use-document-edit-review-overlay.ts, line 49:

<comment>The document remains editable while the review verdict is loading, so users can type after opening a receipt and have those edits presented as part of the AI diff. Disable editability as soon as an active review target exists, and restore it through cleanup for loading, error, and unavailable-result paths.</comment>

<file context>
@@ -0,0 +1,79 @@
+		}
+
+		const review = reviewQuery.data;
+		if (!review) {
+			return;
+		}
</file context>

[cubic-dev-ai]

P2: Navigating between workspace routes can retain the previous workspace's active review and, for a matching item ID, show its diff or submit its receipt IDs under the new workspace. Key the provider by workspace.id (or reset its review state when the workspace changes) to keep review state scoped to one workspace.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/WorkspaceLayout.tsx, line 309:

<comment>Navigating between workspace routes can retain the previous workspace's active review and, for a matching item ID, show its diff or submit its receipt IDs under the new workspace. Key the provider by `workspace.id` (or reset its review state when the workspace changes) to keep review state scoped to one workspace.</comment>

<file context>
@@ -305,11 +306,13 @@ export function WorkspaceShell({
-				) : (
-					workspaceInteractionContent
-				)}
+				<DocumentEditReviewProvider workspaceId={workspace.id}>
+					{hasHeavyViewerRuntimeItems ? (
+						<WorkspacePdfEngineProvider>{workspaceInteractionContent}</WorkspacePdfEngineProvider>
</file context>

Suggested change

	<DocumentEditReviewProvider workspaceId={workspace.id}>
	<DocumentEditReviewProvider key={workspace.id} workspaceId={workspace.id}>

[cubic-dev-ai]

P2: Document-edit receipts can show corrupted line counts when a compact action carries malformed lineChanges: this guard accepts the action, while the grouping code later sums those values directly. Including the optional lineChanges shape in the action contract and validating both fields as numbers would keep malformed activities out of the receipt flow.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/ai-chat/ai-chat-codemode-activity.ts, line 93:

<comment>Document-edit receipts can show corrupted line counts when a compact action carries malformed `lineChanges`: this guard accepts the action, while the grouping code later sums those values directly. Including the optional `lineChanges` shape in the action contract and validating both fields as numbers would keep malformed activities out of the receipt flow.</comment>

<file context>
@@ -77,12 +83,14 @@ function isCompactToolActivity(
 		typeof record.summary === "string" &&
-		typeof record.toolName === "string"
+		typeof record.toolName === "string" &&
+		(action === undefined || isDocumentEditAction(action))
 	);
 }
</file context>

[cubic-dev-ai]

P2: Receipt rows do not show the required persistent “Edited since” or “Undone” state, so the transcript gives no indication when a document has diverged from or already reverted the AI edit. Adding a live row status (or updating the group data to provide one) would preserve that feedback without requiring the user to open each row.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/ai-chat/AiChatDocumentEditActions.tsx, line 43:

<comment>Receipt rows do not show the required persistent “Edited since” or “Undone” state, so the transcript gives no indication when a document has diverged from or already reverted the AI edit. Adding a live row status (or updating the group data to provide one) would preserve that feedback without requiring the user to open each row.</comment>

<file context>
@@ -0,0 +1,107 @@
+			</div>
+			<div className="divide-y divide-foreground/5 border-foreground/5 border-t">
+				{knownGroups.map((group) => (
+					<DocumentEditRow key={group.itemId} group={group} />
+				))}
+			</div>
</file context>

[cubic-dev-ai]

P2: A pending undo can close a different document's review when it finishes. onSuccess unconditionally clears the shared review state, so it should hide the review only when the active item and receipt IDs still match this mutation's target.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/use-document-edit-receipt-undo.ts, line 22:

<comment>A pending undo can close a different document's review when it finishes. `onSuccess` unconditionally clears the shared review state, so it should hide the review only when the active item and receipt IDs still match this mutation's target.</comment>

<file context>
@@ -0,0 +1,42 @@
+			// Every outcome ends the review: it either just undid the changes, or
+			// told us they no longer describe the document. Leaving the marks up
+			// after that would be showing a diff we have just been told is wrong.
+			hideReview();
+
+			if (result.status === "reverted") {
</file context>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

The added outline.totalItems === 0 branch changes the AI context prompt text for empty workspaces but has no test coverage. A straightforward unit test asserting the exact output of formatWorkspaceAiContextOutline for an empty outline is practical and would prevent regressions. Please add a regression test for this branch.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/model/workspace-ai-context-prompt.ts, line 78:

<comment>The added `outline.totalItems === 0` branch changes the AI context prompt text for empty workspaces but has no test coverage. A straightforward unit test asserting the exact output of `formatWorkspaceAiContextOutline` for an empty outline is practical and would prevent regressions. Please add a regression test for this branch.</comment>

<file context>
@@ -73,6 +73,12 @@ export function formatWorkspaceAiContextForPrompt(value: unknown) {
 function formatWorkspaceAiContextOutline(outline: WorkspaceAiContextOutline) {
+	// Said plainly. Reporting "0 items complete" buries an empty workspace in a
+	// sentence whose grammar reads as a truncation notice.
+	if (outline.totalItems === 0) {
+		return ["- Workspace outline: this workspace is empty. It has no items yet."];
+	}
</file context>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

This diff changes when a document collaboration session is marked 'ready' from local IndexedDB persistence: it now additionally requires the tiptap document XML fragment to be non-empty, beyond the existing server-synced marker check. This is a real behavior change that prevents surfacing stale/empty local documents, but it appears to have no test coverage. A regression-style test here is practical — verifying that a persisted ready marker with an empty ydoc fragment does NOT mark the session ready, while a non-empty fragment does, would validate the new guard and protect against future regressions.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/use-document-collaboration-session.ts, line 207:

<comment>This diff changes when a document collaboration session is marked 'ready' from local IndexedDB persistence: it now additionally requires the tiptap document XML fragment to be non-empty, beyond the existing server-synced marker check. This is a real behavior change that prevents surfacing stale/empty local documents, but it appears to have no test coverage. A regression-style test here is practical — verifying that a persisted ready marker with an empty ydoc fragment does NOT mark the session ready, while a non-empty fragment does, would validate the new guard and protect against future regressions.</comment>

<file context>
@@ -203,7 +204,10 @@ function createActiveDocumentSession(input: {
 		.then(() => session.persistence.get(localDocumentReadyKey))
 		.then((wasServerSynced) => {
-			if (wasServerSynced === localDocumentReadyValue) {
+			if (
+				wasServerSynced === localDocumentReadyValue &&
+				session.ydoc.getXmlFragment(tiptapDocumentYjsField).length > 0
</file context>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

Behavior change: after the first item exceeds the batch byte budget, all later items are immediately skipped with read_budget_exceeded even if they would have fit. This changes what the AI reader returns for subsequent documents (e.g., a small document after an oversized one now silently fails). The existing batch-budget test uses 20 identical large documents and does not distinguish the old per-item check from the new early-exit flag, so the new skip-after-exhaustion semantics are untested. Consider adding a regression-style assertion that verifies the early-exit behavior, such as an oversized document followed by a small one.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/content/workspace-content-reader.ts, line 80:

<comment>Behavior change: after the first item exceeds the batch byte budget, all later items are immediately skipped with `read_budget_exceeded` even if they would have fit. This changes what the AI reader returns for subsequent documents (e.g., a small document after an oversized one now silently fails). The existing batch-budget test uses 20 identical large documents and does not distinguish the old per-item check from the new early-exit flag, so the new skip-after-exhaustion semantics are untested. Consider adding a regression-style assertion that verifies the early-exit behavior, such as an oversized document followed by a small one.</comment>

<file context>
@@ -72,6 +71,16 @@ export async function readWorkspaceContent(input: {
+			status: "failed" as const,
+			...(resolution.item.type === "file" ? { type: "file" as const } : {}),
+		};
+		if (readBudgetExhausted) {
+			results.push(readBudgetFailure);
+			continue;
</file context>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

This new hook introduces substantial UI and editor-state behavior (review query handling, toast errors, toggling editor editability, showing/hiding review decorations), but none of it is exercised by tests in the visible diff. For a behavior-change PR, regression-style tests are practical here—for example, asserting that a ready status calls setEditable(false) and showDocumentEditReview, or that an error/unavailable status calls toast.error and hideReview(). Adding tests would prevent silent regressions in the review/undo workflow.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/use-document-edit-review-overlay.ts, line 14:

<comment>This new hook introduces substantial UI and editor-state behavior (review query handling, toast errors, toggling editor editability, showing/hiding review decorations), but none of it is exercised by tests in the visible diff. For a behavior-change PR, regression-style tests are practical here—for example, asserting that a ready status calls `setEditable(false)` and `showDocumentEditReview`, or that an error/unavailable status calls `toast.error` and `hideReview()`. Adding tests would prevent silent regressions in the review/undo workflow.</comment>

<file context>
@@ -0,0 +1,79 @@
+} from "#/features/workspaces/documents/document-edit-review-extension";
+import { documentEditReceiptReviewQueryOptions } from "#/features/workspaces/documents/document-edit-review-queries";
+
+export function useDocumentEditReviewOverlay({
+	canEdit,
+	editor,
</file context>

[cubic-dev-ai]

P3: This flips link-click behavior for the entire collaborative document editor, not just the AI-review read-only surface. openOnClick: true on the shared schema means a plain click on any link in any document now navigates the user away, which interferes with placing the cursor or selecting text around a link during normal editing (the server-side AI serialization never reads this setting, so it only affects live editing). If the intent was to let links open in the review view, consider scoping that to the review/decoration layer instead of changing the interactive editor for all documents.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/tiptap-schema.ts, line 78:

<comment>This flips link-click behavior for the entire collaborative document editor, not just the AI-review read-only surface. `openOnClick: true` on the shared schema means a plain click on any link in any document now navigates the user away, which interferes with placing the cursor or selecting text around a link during normal editing (the server-side AI serialization never reads this setting, so it only affects live editing). If the intent was to let links open in the review view, consider scoping that to the review/decoration layer instead of changing the interactive editor for all documents.</comment>

<file context>
@@ -40,7 +75,7 @@ export function getTiptapDocumentSchemaExtensions({
 		Highlight,
 		Link.configure({
-			openOnClick: false,
+			openOnClick: true,
 			autolink: true,
 			defaultProtocol: "https",
</file context>

Suggested change

	openOnClick: true,
	openOnClick: false,

[cubic-dev-ai]

17 issues found across 54 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/features/workspaces/components/ai-chat/AiChatMessageRow.tsx">

<violation number="1" location="src/features/workspaces/components/ai-chat/AiChatMessageRow.tsx:62">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The new document-edit receipt rendering logic added to AiChatMessageRow lacks test coverage. The component now conditionally renders AiChatDocumentEditActions based on isAssistant, display kind, and streaming state, but no tests verify that these receipts appear for assistant messages, are suppressed during streaming, or are hidden for user messages. Given that this is a bug-fix or behavior-change PR, please add regression-style tests for this new render behavior so the claimed behavior is actually exercised and protected against future regressions.</violation>

<violation number="2" location="src/features/workspaces/components/ai-chat/AiChatMessageRow.tsx:64">
P2: Legacy assistant turns that use the sibling-tool fallback never show their document edit receipt, so users cannot open the review/undo workflow for those edits. Preserving the document-edit action in the fallback or extracting it from the original tool parts would keep those receipts available.</violation>
</file>

<file name="src/features/workspaces/content/workspace-content-reader.ts">

<violation number="1" location="src/features/workspaces/content/workspace-content-reader.ts:98">
P1: A single oversized document now suppresses every later request, so a large document can prevent the model from reading unrelated smaller documents that still fit within the batch budget. Continuing to evaluate later requests against `returnedContentBytes` preserves the per-batch limit without dropping otherwise readable context.</violation>
</file>

<file name="src/features/workspaces/components/ai-chat/AiChatDocumentEditActions.tsx">

<violation number="1" location="src/features/workspaces/components/ai-chat/AiChatDocumentEditActions.tsx:17">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

This new behavior-change UI component introduces user-facing interactions (filtering deleted documents, click-to-review, toast fallback, and conditional line-change tallies) without any tests. The rule flags behavior-change PRs where changed behavior is not exercised by tests when regression-style assertions are practical. Consider adding tests that verify: deleted documents are filtered from the list, clicking a row invokes `showReview` with the correct arguments, a missing document triggers the toast error, and `ChangeSummary` renders only when there are actual line changes.</violation>

<violation number="2" location="src/features/workspaces/components/ai-chat/AiChatDocumentEditActions.tsx:38">
P2: After undoing an AI edit, its transcript receipt still says `Edited` and shows the original tally, so users cannot tell which changes remain actionable without reopening each row. The receipt should track and render the live receipt status, including `Undone`/unavailable states, and update it after the undo or review check.</violation>

<violation number="3" location="src/features/workspaces/components/ai-chat/AiChatDocumentEditActions.tsx:57">
P2: This receipt row renders block-level <div> elements inside a <button>, which is invalid HTML nesting (a button's content model allows only phrasing content). React logs a validateDOMNesting console warning for this pattern. The row's visual layout can be preserved while keeping the clickable semantic by swapping the inner <div> containers for <span> elements (with block/flex display as needed), so the surrounding markup stays conforming.</violation>
</file>

<file name="src/features/workspaces/components/document-editor/DocumentEditorSurface.tsx">

<violation number="1" location="src/features/workspaces/components/document-editor/DocumentEditorSurface.tsx:109">
P2: Opening a receipt leaves an editable document during the server-verdict loading window, even though the UI already says “Reviewing changes.” A keystroke in that window can be persisted as a user edit and make the review fail with `content_changed`; disable editing as soon as the review becomes active and restore it only when the review ends or is rejected.</violation>

<violation number="2" location="src/features/workspaces/components/document-editor/DocumentEditorSurface.tsx:109">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

This behavior-change PR introduces new UI-level review and undo behavior via `useDocumentEditReviewOverlay` and `DocumentEditReviewExtension`, but I could not find any tests that exercise this overlay hook or the review extension. While the underlying AI edit application logic is tested in `document-ai-edits.test.ts`, the React hook and TipTap extension wiring that enable inline review marks, read-only review mode, and the undo toolbar appear untested. Consider adding tests that assert the overlay initializes correctly, decorations render for insertions/deletions, and the undo confirmation flow behaves as expected.</violation>
</file>

<file name="src/features/workspaces/documents/document-edit-review-queries.ts">

<violation number="1" location="src/features/workspaces/documents/document-edit-review-queries.ts:24">
P2: Reopening a receipt can show the old AI diff on the user's later document because `staleTime: 0` marks data stale but does not prevent cached data from being rendered during the background refetch. Configure actual no-retention/per-open refresh behavior, such as `gcTime: 0` here and/or clearing or ignoring prior data until the current server verdict arrives.</violation>
</file>

<file name="src/features/workspaces/documents/document-edit-review-functions.ts">

<violation number="1" location="src/features/workspaces/documents/document-edit-review-functions.ts:23">
P2: A valid receipt group can fail before this handler runs: GET server functions place the serialized `data` in the request URL, while this schema accepts up to 40 IDs of 512 characters each. Using POST for the review RPC would let large transcript edit groups reach the server instead of hitting the Cloudflare URL limit.</violation>
</file>

<file name="src/features/workspaces/operations/workspace-tool-schemas.ts">

<violation number="1" location="src/features/workspaces/operations/workspace-tool-schemas.ts:336">
P2: Create-item results can now validate unsupported item types such as `file`, `flashcard`, and `quiz`, weakening the tool contract and allowing malformed results to pass output validation. Keep the create result's `type` restricted to `"folder" | "document"`.</violation>
</file>

<file name="src/features/workspaces/documents/document-ai-html.ts">

<violation number="1" location="src/features/workspaces/documents/document-ai-html.ts:214">
P2: Malformed math HTML can be accepted while its visible contents are silently lost; require `data-latex` and reject child content for both inline and block math before schema parsing. Otherwise an AI edit containing fallback text inside a math element reports success but changes the document to an empty formula.</violation>
</file>

<file name="src/features/workspaces/documents/document-edit-review-extension.ts">

<violation number="1" location="src/features/workspaces/documents/document-edit-review-extension.ts:76">
P2: While a review is open the document is read-only, so local typing is blocked — but this still runs on every remote/collaboration transaction that changes the doc. Each such transaction triggers a full-document ChangeSet token diff plus recreation of every deletion-widget DOM node, which can cause noticeable jank on large documents that others are actively editing. Consider adding a cheap guard (e.g. skip recompute when the change range doesn't overlap review-relevant content) or debouncing, so held-open reviews don't pay a full-doc diff on every keystroke from a collaborator.</violation>
</file>

<file name="src/features/workspaces/components/document-editor/DocumentToolbar.tsx">

<violation number="1" location="src/features/workspaces/components/document-editor/DocumentToolbar.tsx:66">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The comment claims the read-only toolbar shows 'only what still works,' but the rendered `DocumentMoreMenu` and `DocumentExportMenuGroup` contain exclusively disabled placeholder items ('PDF' and 'Google Docs' both labeled 'Soon'). No functional controls are actually presented, making the comment misleading and asserting behavior the code does not implement.</violation>
</file>

<file name="src/features/workspaces/ai/ai-thread-tool-ui-metadata.ts">

<violation number="1" location="src/features/workspaces/ai/ai-thread-tool-ui-metadata.ts:7">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The new AI tool metadata utility functions are not covered by unit tests, despite being straightforward to test and central to the PR's behavior change. Consider adding a test file (e.g., `ai-thread-tool-ui-metadata.test.ts`) with at least a round-trip assertion: attach a receipt ID, retrieve it, strip it recursively, and assert the metadata is removed.</violation>
</file>

<file name="src/features/workspaces/documents/use-document-collaboration-session.ts">

<violation number="1" location="src/features/workspaces/documents/use-document-collaboration-session.ts:207">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

This behavior change introduces a new readiness gate—`session.ydoc.getXmlFragment(tiptapDocumentYjsField).length > 0`—in the local-persistence restore path, but it isn’t exercised by any tests. If a previously synced document happens to have an empty TipTap fragment, restoring from cache will now leave the session permanently unready while offline. Consider adding a regression test that seeds IndexedDB with `server-synced=true` and an empty fragment and asserts `ready` stays false until the fragment is populated.</violation>
</file>

<file name="src/features/workspaces/components/ai-chat/ai-chat-codemode-activity.ts">

<violation number="1" location="src/features/workspaces/components/ai-chat/ai-chat-codemode-activity.ts:191">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The newly added `isDocumentEditAction` type guard and its use in `isCompactToolActivity` introduce behavior that validates the shape of server-provided `action` records, yet no tests exercise this validation path. In a behavior-change PR, regression-style assertions for this pure type-guard logic would be practical and help prevent silently accepting malformed records or incorrectly rejecting valid ones.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P1: A single oversized document now suppresses every later request, so a large document can prevent the model from reading unrelated smaller documents that still fit within the batch budget. Continuing to evaluate later requests against returnedContentBytes preserves the per-batch limit without dropping otherwise readable context.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/content/workspace-content-reader.ts, line 98:

<comment>A single oversized document now suppresses every later request, so a large document can prevent the model from reading unrelated smaller documents that still fit within the batch budget. Continuing to evaluate later requests against `returnedContentBytes` preserves the per-batch limit without dropping otherwise readable context.</comment>

<file context>
@@ -86,20 +95,16 @@ export async function readWorkspaceContent(input: {
-					status: "failed",
-					...(resolution.item.type === "file" ? { type: "file" as const } : {}),
-				});
+				readBudgetExhausted = true;
+				results.push(readBudgetFailure);
 				continue;
</file context>

[cubic-dev-ai]

P2: Legacy assistant turns that use the sibling-tool fallback never show their document edit receipt, so users cannot open the review/undo workflow for those edits. Preserving the document-edit action in the fallback or extracting it from the original tool parts would keep those receipts available.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/ai-chat/AiChatMessageRow.tsx, line 64:

<comment>Legacy assistant turns that use the sibling-tool fallback never show their document edit receipt, so users cannot open the review/undo workflow for those edits. Preserving the document-edit action in the fallback or extracting it from the original tool parts would keep those receipts available.</comment>

<file context>
@@ -57,6 +59,10 @@ export default function AiChatMessageRow({
 	const copyableText = !isStreaming ? getCopyableMessageText(message) : "";
+	const documentEditGroups =
+		isAssistant && display?.kind === "content" && !isStreaming
+			? getAiChatDocumentEditGroups(display.parts)
+			: [];
 
</file context>

[cubic-dev-ai]

P2: After undoing an AI edit, its transcript receipt still says Edited and shows the original tally, so users cannot tell which changes remain actionable without reopening each row. The receipt should track and render the live receipt status, including Undone/unavailable states, and update it after the undo or review check.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/ai-chat/AiChatDocumentEditActions.tsx, line 38:

<comment>After undoing an AI edit, its transcript receipt still says `Edited` and shows the original tally, so users cannot tell which changes remain actionable without reopening each row. The receipt should track and render the live receipt status, including `Undone`/unavailable states, and update it after the undo or review check.</comment>

<file context>
@@ -0,0 +1,107 @@
+			<div className="flex items-center gap-1.5 px-2.5 py-1.5 text-muted-foreground text-xs">
+				<FilePen className="size-3 shrink-0" aria-hidden="true" />
+				<span className="font-medium">
+					Edited {knownGroups.length} {knownGroups.length === 1 ? "document" : "documents"}
+				</span>
+			</div>
</file context>

[cubic-dev-ai]

P2: Opening a receipt leaves an editable document during the server-verdict loading window, even though the UI already says “Reviewing changes.” A keystroke in that window can be persisted as a user edit and make the review fail with content_changed; disable editing as soon as the review becomes active and restore it only when the review ends or is rejected.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/document-editor/DocumentEditorSurface.tsx, line 109:

<comment>Opening a receipt leaves an editable document during the server-verdict loading window, even though the UI already says “Reviewing changes.” A keystroke in that window can be persisted as a user edit and make the review fail with `content_changed`; disable editing as soon as the review becomes active and restore it only when the review ends or is rejected.</comment>

<file context>
@@ -98,7 +100,18 @@ function DocumentEditorInstance({
+		itemId: item.id,
+		slotId: viewInstanceId,
+	});
+	useDocumentEditReviewOverlay({
+		canEdit: capabilities.canMutateContent,
+		editor,
</file context>

[cubic-dev-ai]

P2: Reopening a receipt can show the old AI diff on the user's later document because staleTime: 0 marks data stale but does not prevent cached data from being rendered during the background refetch. Configure actual no-retention/per-open refresh behavior, such as gcTime: 0 here and/or clearing or ignoring prior data until the current server verdict arrives.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/document-edit-review-queries.ts, line 24:

<comment>Reopening a receipt can show the old AI diff on the user's later document because `staleTime: 0` marks data stale but does not prevent cached data from being rendered during the background refetch. Configure actual no-retention/per-open refresh behavior, such as `gcTime: 0` here and/or clearing or ignoring prior data until the current server verdict arrives.</comment>

<file context>
@@ -0,0 +1,26 @@
+		// whether the document still matches the receipt, and a verdict cached
+		// from an earlier open would mark the reader's own later writing as the
+		// assistant's.
+		staleTime: 0,
+	});
+}
</file context>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

The new AI tool metadata utility functions are not covered by unit tests, despite being straightforward to test and central to the PR's behavior change. Consider adding a test file (e.g., ai-thread-tool-ui-metadata.test.ts) with at least a round-trip assertion: attach a receipt ID, retrieve it, strip it recursively, and assert the metadata is removed.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/ai/ai-thread-tool-ui-metadata.ts, line 7:

<comment>The new AI tool metadata utility functions are not covered by unit tests, despite being straightforward to test and central to the PR's behavior change. Consider adding a test file (e.g., `ai-thread-tool-ui-metadata.test.ts`) with at least a round-trip assertion: attach a receipt ID, retrieve it, strip it recursively, and assert the metadata is removed.</comment>

<file context>
@@ -0,0 +1,52 @@
+	documentEditReceiptId?: string;
+}
+
+export function attachDocumentEditReceiptMetadata(output: unknown, receiptId: string) {
+	if (!isPlainRecord(output)) {
+		return output;
</file context>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

This behavior change introduces a new readiness gate—session.ydoc.getXmlFragment(tiptapDocumentYjsField).length > 0—in the local-persistence restore path, but it isn’t exercised by any tests. If a previously synced document happens to have an empty TipTap fragment, restoring from cache will now leave the session permanently unready while offline. Consider adding a regression test that seeds IndexedDB with server-synced=true and an empty fragment and asserts ready stays false until the fragment is populated.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/use-document-collaboration-session.ts, line 207:

<comment>This behavior change introduces a new readiness gate—`session.ydoc.getXmlFragment(tiptapDocumentYjsField).length > 0`—in the local-persistence restore path, but it isn’t exercised by any tests. If a previously synced document happens to have an empty TipTap fragment, restoring from cache will now leave the session permanently unready while offline. Consider adding a regression test that seeds IndexedDB with `server-synced=true` and an empty fragment and asserts `ready` stays false until the fragment is populated.</comment>

<file context>
@@ -203,7 +204,10 @@ function createActiveDocumentSession(input: {
 		.then(() => session.persistence.get(localDocumentReadyKey))
 		.then((wasServerSynced) => {
-			if (wasServerSynced === localDocumentReadyValue) {
+			if (
+				wasServerSynced === localDocumentReadyValue &&
+				session.ydoc.getXmlFragment(tiptapDocumentYjsField).length > 0
</file context>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

The newly added isDocumentEditAction type guard and its use in isCompactToolActivity introduce behavior that validates the shape of server-provided action records, yet no tests exercise this validation path. In a behavior-change PR, regression-style assertions for this pure type-guard logic would be practical and help prevent silently accepting malformed records or incorrectly rejecting valid ones.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/ai-chat/ai-chat-codemode-activity.ts, line 191:

<comment>The newly added `isDocumentEditAction` type guard and its use in `isCompactToolActivity` introduce behavior that validates the shape of server-provided `action` records, yet no tests exercise this validation path. In a behavior-change PR, regression-style assertions for this pure type-guard logic would be practical and help prevent silently accepting malformed records or incorrectly rejecting valid ones.</comment>

<file context>
@@ -179,3 +187,19 @@ function isCallState(value: unknown): value is ToolLogEntry["state"] {
 	return value === "completed" || value === "failed" || value === "running";
 }
+
+function isDocumentEditAction(
+	value: unknown,
+): value is NonNullable<AiChatToolChildActivity["action"]> {
</file context>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

The new document-edit receipt rendering logic added to AiChatMessageRow lacks test coverage. The component now conditionally renders AiChatDocumentEditActions based on isAssistant, display kind, and streaming state, but no tests verify that these receipts appear for assistant messages, are suppressed during streaming, or are hidden for user messages. Given that this is a bug-fix or behavior-change PR, please add regression-style tests for this new render behavior so the claimed behavior is actually exercised and protected against future regressions.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/ai-chat/AiChatMessageRow.tsx, line 62:

<comment>The new document-edit receipt rendering logic added to AiChatMessageRow lacks test coverage. The component now conditionally renders AiChatDocumentEditActions based on isAssistant, display kind, and streaming state, but no tests verify that these receipts appear for assistant messages, are suppressed during streaming, or are hidden for user messages. Given that this is a bug-fix or behavior-change PR, please add regression-style tests for this new render behavior so the claimed behavior is actually exercised and protected against future regressions.</comment>

<file context>
@@ -57,6 +59,10 @@ export default function AiChatMessageRow({
 		? []
 		: displayableParts.filter((part) => !isAttachmentPart(part));
 	const copyableText = !isStreaming ? getCopyableMessageText(message) : "";
+	const documentEditGroups =
+		isAssistant && display?.kind === "content" && !isStreaming
+			? getAiChatDocumentEditGroups(display.parts)
</file context>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

This new behavior-change UI component introduces user-facing interactions (filtering deleted documents, click-to-review, toast fallback, and conditional line-change tallies) without any tests. The rule flags behavior-change PRs where changed behavior is not exercised by tests when regression-style assertions are practical. Consider adding tests that verify: deleted documents are filtered from the list, clicking a row invokes showReview with the correct arguments, a missing document triggers the toast error, and ChangeSummary renders only when there are actual line changes.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/ai-chat/AiChatDocumentEditActions.tsx, line 17:

<comment>This new behavior-change UI component introduces user-facing interactions (filtering deleted documents, click-to-review, toast fallback, and conditional line-change tallies) without any tests. The rule flags behavior-change PRs where changed behavior is not exercised by tests when regression-style assertions are practical. Consider adding tests that verify: deleted documents are filtered from the list, clicking a row invokes `showReview` with the correct arguments, a missing document triggers the toast error, and `ChangeSummary` renders only when there are actual line changes.</comment>

<file context>
@@ -0,0 +1,107 @@
+ * reviewed or undone is a live question, and answering it up front would cost a
+ * request per row on every chat load just to render a label. Clicking finds out.
+ */
+export function AiChatDocumentEditActions({
+	groups,
+}: {
</file context>

[cubic-dev-ai]

15 issues found and verified against the latest diff

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/features/workspaces/documents/document-html-chunk.ts">

<violation number="1" location="src/features/workspaces/documents/document-html-chunk.ts:41">
P3: Every chunk read runs a full, expensive serialization (SHA-256 fingerprint + DOMSerializer pass) on the boundary block only to discover it won't fit, then discards that work and `break`s. For a large document read in many chunks, that's one wasted crypto+DOM serialization per chunk. Consider determining the boundary with a cheaper length estimate (e.g. node text length or a rough size projection) before doing the full serialization, or otherwise avoid discarding an already-serialized block. Low impact, but worth considering since serialization here is non-trivial.</violation>
</file>

<file name="src/features/workspaces/components/document-editor/DocumentEditorSurface.tsx">

<violation number="1" location="src/features/workspaces/components/document-editor/DocumentEditorSurface.tsx:92">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

New user-facing behaviors added in this diff lack test coverage. Specifically, the handleDOMEvents.click citation navigation handler and the useDocumentEditReviewOverlay / DocumentEditReviewExtension review overlay are not exercised by any visible tests. Rule 1 flags behavior-change PRs where the changed behavior is not tested, especially when regression-style assertions are practical (e.g. simulating a citation click and asserting reveal is called with the expected arguments, or asserting the review overlay mounts correctly). Consider adding unit or component tests that cover these interactions to prevent regressions.</violation>

<violation number="2" location="src/features/workspaces/components/document-editor/DocumentEditorSurface.tsx:103">
P3: Clicking a citation that points at a source that no longer exists (or isn't in the current workspace) silently does nothing: the handler ignores reveal()'s boolean return and always prevents the default. Since citations are meant to read as prose even after a source is deleted, guard the navigation with hasItem() (added to the location context in this same PR) so a stale citation doesn't present a dead, no-feedback click, or else provide a fallback message when reveal fails.</violation>
</file>

<file name="src/features/workspaces/ai/ai-thread-tool-ui-metadata.ts">

<violation number="1" location="src/features/workspaces/ai/ai-thread-tool-ui-metadata.ts:45">
P2: Completed orchestration results can lose legitimate data whenever they contain a `__thinkexUi` property. The filter should use metadata that cannot collide with result data, or remove only metadata known to have been attached to document-edit tool results instead of deleting every matching key recursively.</violation>
</file>

<file name="src/features/workspaces/operations/workspace-tool-schemas.ts">

<violation number="1" location="src/features/workspaces/operations/workspace-tool-schemas.ts:336">
P2: Create results now advertise every workspace item type even though this tool only creates folders and documents, weakening the output contract and allowing an invalid producer result to pass validation. Keep the create-specific `type` enum narrowed to `document` and `folder`.</violation>
</file>

<file name="src/features/workspaces/operations/create-items.ts">

<violation number="1" location="src/features/workspaces/operations/create-items.ts:125">
P2: Citations using an accepted but non-canonical absolute path are silently stored as plain text instead of linked workspace citations. Normalizing the citation path before lookup, or preserving a raw-to-normalized path mapping, keeps resolved citations linked.</violation>

<violation number="2" location="src/features/workspaces/operations/create-items.ts:125">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The new `await resolveDocumentCitations(...)` call is performed outside the existing `try/catch` in `getCreateWorkspaceItemInitialContent`, so if citation resolution rejects (for example, due to a kernel error), the entire `createWorkspaceItemsOperation` batch will throw instead of gracefully recording a per-item `invalid_initial_content` failure as the old Markdown parse path did. This changed error-handling behavior is not covered by any tests — searches for `createWorkspaceItemsOperation` and `resolveDocumentCitations` found zero test references. Per the "Flag AI Slop and Fabricated Changes" rule, behavior-change PRs should exercise changed behavior with tests, especially when a practical regression assertion is possible. Consider either moving citation resolution inside `getCreateWorkspaceItemInitialContent` so failures are caught as `invalid_initial_content`, or adding tests that verify the intended failure mode.</violation>
</file>

<file name="src/features/workspaces/components/ai-chat/ai-chat-codemode-activity.ts">

<violation number="1" location="src/features/workspaces/components/ai-chat/ai-chat-codemode-activity.ts:16">
P3: The client-side `action` type on `AiChatToolChildActivity` omits `lineChanges`, even though the server's orchestration contract includes it (optionally) and the codemode group path in `getAiChatDocumentEditGroups` reads `action.lineChanges` to compute the Lines: +N −M tally shown in the receipt. Right now it works only because the server happens to attach the field and the consumer's parameter type leaves it optional; the declared source interface doesn't reflect the actual payload shape. Aligning the type with the server contract (and the consumer) would make the line-tally path type-truthful and less fragile to future refactors or stricter typing.</violation>
</file>

<file name="src/features/workspaces/content/workspace-content-reader.ts">

<violation number="1" location="src/features/workspaces/content/workspace-content-reader.ts:80">
P2: This new `readBudgetExhausted` short-circuit changes batch-read semantics in a way that can lose requested content in mixed-size batches. Previously a single over-budget item was failed but the loop kept going, so a later small document that fit within the remaining byte budget was still returned ready. Now the first over-budget item causes every subsequent unread item — including small documents that would fit — to be marked `read_budget_exceeded` with a misleading 'too big' signal. The model then has to re-issue reads for those items across extra turns. If the intent is a clean fail-fast prefix, consider re-reading/returning items that fit after an oversize gap, or document and intentionally test the mixed-size behavior.</violation>

<violation number="2" location="src/features/workspaces/content/workspace-content-reader.ts:98">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The read-budget short-circuit behavior and deferred parallel relation resolution are behavior changes that lack regression tests. The batch-budget test in `workspace-content-reader.test.ts` uses 20 oversized documents that each individually exceed the budget, so it passes under both the old and new semantics and does not verify that subsequent smaller items are now skipped once the budget is exhausted. Consider adding a test with mixed-size items (one over-budget followed by one under-budget) to pin down the short-circuit semantics, and exercise the deferred `Promise.all` resolution of `listItemRelations` with actual relation data.</violation>
</file>

<file name="src/features/workspaces/documents/tiptap-schema.ts">

<violation number="1" location="src/features/workspaces/documents/tiptap-schema.ts:41">
P2: Model-authored citations can carry a `data-item-id` directly and skip the server-side path-resolution step entirely. The resolution flow (`resolveDocumentCitations` -> `applyDocumentCitationItemIds`) is the only place that is supposed to assign item ids, but `parseHTML` here also matches `citation[data-item-id]`, and `parseDocumentAiHtml` preserves any citation that already has one. A model (or crafted HTML) can therefore anchor a citation to an arbitrary item id without it being resolved or validated, weakening the provenance guarantee that comments in this file describe. Consider parsing citations only from a neutral form (e.g. `path`/`page`) in the schema and rejecting or neutralizing a raw `data-item-id` in model input, so item ids are assigned exclusively by the resolution step.</violation>
</file>

<file name="src/features/workspaces/documents/use-document-edit-receipt-undo.ts">

<violation number="1" location="src/features/workspaces/documents/use-document-edit-receipt-undo.ts:13">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

This new hook introduces user-visible behavior (per-status toast messages, review dismissal on success, and error handling) but appears to have no test coverage. The PR is a behavior-change PR, and regression-style assertions are practical—for example, verifying that `reverted` shows 'Changes undone.' and calls `hideReview()`, or that each unavailable status maps to its correct toast message. Consider adding unit or integration tests for this hook or the underlying mutation to prevent regressions.</violation>

<violation number="2" location="src/features/workspaces/documents/use-document-edit-receipt-undo.ts:14">
P2: `hideReview()` here is the global context action, so it clears whichever review is active in the reader — not necessarily the one the undo call corresponds to. Because this hook is shared by the chat receipt rows and the review toolbar, undoing an edit from a chat receipt for one document will also dismiss the review currently open on an unrelated document, dropping its marks without the user intending to. It would be safer to hide only the review that matches this target's `itemId`/`receiptIds` (or have `hideReview` take the target and no-op on mismatch).</violation>
</file>

<file name="src/features/workspaces/components/ai-chat/ai-chat-document-edit-actions.ts">

<violation number="1" location="src/features/workspaces/components/ai-chat/ai-chat-document-edit-actions.ts:20">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The new `getAiChatDocumentEditGroups` function implements non-trivial filtering, deduplication by `receiptId`, and `lineChanges` aggregation, yet there is no dedicated unit test exercising it. On a behavior-change PR that introduces a reviewable/undoable edit workflow, this logic should have regression-style test coverage to guard against silent breakages in grouping or summation.</violation>
</file>

<file name="src/features/workspaces/operations/document-citations.ts">

<violation number="1" location="src/features/workspaces/operations/document-citations.ts:14">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

This PR introduces new behavior in `resolveDocumentCitations` that rewrites document HTML citation paths to item IDs, but no tests for this function or its helpers appear in the diff or elsewhere in the codebase. The custom rule calls out behavior-change PRs where new behavior isn't exercised by tests—especially when a straightforward regression-style assertion is practical. Adding a small unit test (e.g., passing HTML with citation paths, mocking the kernel's `resolvePaths`, and asserting the returned HTML contains the resolved item IDs) would back up the claimed behavior and prevent regressions.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: Completed orchestration results can lose legitimate data whenever they contain a __thinkexUi property. The filter should use metadata that cannot collide with result data, or remove only metadata known to have been attached to document-edit tool results instead of deleting every matching key recursively.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/ai/ai-thread-tool-ui-metadata.ts, line 45:

<comment>Completed orchestration results can lose legitimate data whenever they contain a `__thinkexUi` property. The filter should use metadata that cannot collide with result data, or remove only metadata known to have been attached to document-edit tool results instead of deleting every matching key recursively.</comment>

<file context>
@@ -0,0 +1,52 @@
+
+	return Object.fromEntries(
+		Object.entries(value).flatMap(([key, entry]) =>
+			key === aiThreadToolUiMetadataKey ? [] : [[key, stripAIThreadToolUiMetadata(entry)]],
+		),
+	);
</file context>

[cubic-dev-ai]

P2: Create results now advertise every workspace item type even though this tool only creates folders and documents, weakening the output contract and allowing an invalid producer result to pass validation. Keep the create-specific type enum narrowed to document and folder.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/operations/workspace-tool-schemas.ts, line 336:

<comment>Create results now advertise every workspace item type even though this tool only creates folders and documents, weakening the output contract and allowing an invalid producer result to pass validation. Keep the create-specific `type` enum narrowed to `document` and `folder`.</comment>

<file context>
@@ -318,15 +333,7 @@ export const workspaceListItemsOutputSchema = z.object({
-			.optional()
-			.describe("Content projection warnings for created documents."),
-	}),
+	itemSchema: workspacePathItemSchema.extend({ itemId: z.string().min(1) }),
 	failureSchema: createFailureSchema(createWorkspaceItemsFailureCodes),
 }).extend({
</file context>

Suggested change

	itemSchema: workspacePathItemSchema.extend({ itemId: z.string().min(1) }),
	itemSchema: workspacePathItemSchema.extend({
	type: z.enum(["document", "folder"]),
	itemId: z.string().min(1),
	}),

[cubic-dev-ai]

P2: Citations using an accepted but non-canonical absolute path are silently stored as plain text instead of linked workspace citations. Normalizing the citation path before lookup, or preserving a raw-to-normalized path mapping, keeps resolved citations linked.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/operations/create-items.ts, line 125:

<comment>Citations using an accepted but non-canonical absolute path are silently stored as plain text instead of linked workspace citations. Normalizing the citation path before lookup, or preserving a raw-to-normalized path mapping, keeps resolved citations linked.</comment>

<file context>
@@ -118,7 +118,17 @@ export async function createWorkspaceItemsOperation(
+			itemInput.type === "document" && itemInput.initialContent !== undefined
+				? {
+						...itemInput,
+						initialContent: await resolveDocumentCitations({
+							html: itemInput.initialContent,
+							kernel,
</file context>

[cubic-dev-ai]

P2: This new readBudgetExhausted short-circuit changes batch-read semantics in a way that can lose requested content in mixed-size batches. Previously a single over-budget item was failed but the loop kept going, so a later small document that fit within the remaining byte budget was still returned ready. Now the first over-budget item causes every subsequent unread item — including small documents that would fit — to be marked read_budget_exceeded with a misleading 'too big' signal. The model then has to re-issue reads for those items across extra turns. If the intent is a clean fail-fast prefix, consider re-reading/returning items that fit after an oversize gap, or document and intentionally test the mixed-size behavior.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/content/workspace-content-reader.ts, line 80:

<comment>This new `readBudgetExhausted` short-circuit changes batch-read semantics in a way that can lose requested content in mixed-size batches. Previously a single over-budget item was failed but the loop kept going, so a later small document that fit within the remaining byte budget was still returned ready. Now the first over-budget item causes every subsequent unread item — including small documents that would fit — to be marked `read_budget_exceeded` with a misleading 'too big' signal. The model then has to re-issue reads for those items across extra turns. If the intent is a clean fail-fast prefix, consider re-reading/returning items that fit after an oversize gap, or document and intentionally test the mixed-size behavior.</comment>

<file context>
@@ -72,6 +71,16 @@ export async function readWorkspaceContent(input: {
+			status: "failed" as const,
+			...(resolution.item.type === "file" ? { type: "file" as const } : {}),
+		};
+		if (readBudgetExhausted) {
+			results.push(readBudgetFailure);
+			continue;
</file context>

[cubic-dev-ai]

P2: Model-authored citations can carry a data-item-id directly and skip the server-side path-resolution step entirely. The resolution flow (resolveDocumentCitations -> applyDocumentCitationItemIds) is the only place that is supposed to assign item ids, but parseHTML here also matches citation[data-item-id], and parseDocumentAiHtml preserves any citation that already has one. A model (or crafted HTML) can therefore anchor a citation to an arbitrary item id without it being resolved or validated, weakening the provenance guarantee that comments in this file describe. Consider parsing citations only from a neutral form (e.g. path/page) in the schema and rejecting or neutralizing a raw data-item-id in model input, so item ids are assigned exclusively by the resolution step.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/tiptap-schema.ts, line 41:

<comment>Model-authored citations can carry a `data-item-id` directly and skip the server-side path-resolution step entirely. The resolution flow (`resolveDocumentCitations` -> `applyDocumentCitationItemIds`) is the only place that is supposed to assign item ids, but `parseHTML` here also matches `citation[data-item-id]`, and `parseDocumentAiHtml` preserves any citation that already has one. A model (or crafted HTML) can therefore anchor a citation to an arbitrary item id without it being resolved or validated, weakening the provenance guarantee that comments in this file describe. Consider parsing citations only from a neutral form (e.g. `path`/`page`) in the schema and rejecting or neutralizing a raw `data-item-id` in model input, so item ids are assigned exclusively by the resolution step.</comment>

<file context>
@@ -12,6 +12,82 @@ import StarterKit from "@tiptap/starter-kit";
+		};
+	},
+
+	parseHTML() {
+		return [{ tag: "citation[data-item-id]" }];
+	},
</file context>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

The new await resolveDocumentCitations(...) call is performed outside the existing try/catch in getCreateWorkspaceItemInitialContent, so if citation resolution rejects (for example, due to a kernel error), the entire createWorkspaceItemsOperation batch will throw instead of gracefully recording a per-item invalid_initial_content failure as the old Markdown parse path did. This changed error-handling behavior is not covered by any tests — searches for createWorkspaceItemsOperation and resolveDocumentCitations found zero test references. Per the "Flag AI Slop and Fabricated Changes" rule, behavior-change PRs should exercise changed behavior with tests, especially when a practical regression assertion is possible. Consider either moving citation resolution inside getCreateWorkspaceItemInitialContent so failures are caught as invalid_initial_content, or adding tests that verify the intended failure mode.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/operations/create-items.ts, line 125:

<comment>The new `await resolveDocumentCitations(...)` call is performed outside the existing `try/catch` in `getCreateWorkspaceItemInitialContent`, so if citation resolution rejects (for example, due to a kernel error), the entire `createWorkspaceItemsOperation` batch will throw instead of gracefully recording a per-item `invalid_initial_content` failure as the old Markdown parse path did. This changed error-handling behavior is not covered by any tests — searches for `createWorkspaceItemsOperation` and `resolveDocumentCitations` found zero test references. Per the "Flag AI Slop and Fabricated Changes" rule, behavior-change PRs should exercise changed behavior with tests, especially when a practical regression assertion is possible. Consider either moving citation resolution inside `getCreateWorkspaceItemInitialContent` so failures are caught as `invalid_initial_content`, or adding tests that verify the intended failure mode.</comment>

<file context>
@@ -118,7 +118,17 @@ export async function createWorkspaceItemsOperation(
+			itemInput.type === "document" && itemInput.initialContent !== undefined
+				? {
+						...itemInput,
+						initialContent: await resolveDocumentCitations({
+							html: itemInput.initialContent,
+							kernel,
</file context>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

The read-budget short-circuit behavior and deferred parallel relation resolution are behavior changes that lack regression tests. The batch-budget test in workspace-content-reader.test.ts uses 20 oversized documents that each individually exceed the budget, so it passes under both the old and new semantics and does not verify that subsequent smaller items are now skipped once the budget is exhausted. Consider adding a test with mixed-size items (one over-budget followed by one under-budget) to pin down the short-circuit semantics, and exercise the deferred Promise.all resolution of listItemRelations with actual relation data.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/content/workspace-content-reader.ts, line 98:

<comment>The read-budget short-circuit behavior and deferred parallel relation resolution are behavior changes that lack regression tests. The batch-budget test in `workspace-content-reader.test.ts` uses 20 oversized documents that each individually exceed the budget, so it passes under both the old and new semantics and does not verify that subsequent smaller items are now skipped once the budget is exhausted. Consider adding a test with mixed-size items (one over-budget followed by one under-budget) to pin down the short-circuit semantics, and exercise the deferred `Promise.all` resolution of `listItemRelations` with actual relation data.</comment>

<file context>
@@ -86,20 +95,16 @@ export async function readWorkspaceContent(input: {
-					status: "failed",
-					...(resolution.item.type === "file" ? { type: "file" as const } : {}),
-				});
+				readBudgetExhausted = true;
+				results.push(readBudgetFailure);
 				continue;
</file context>

[cubic-dev-ai]

P3: Every chunk read runs a full, expensive serialization (SHA-256 fingerprint + DOMSerializer pass) on the boundary block only to discover it won't fit, then discards that work and breaks. For a large document read in many chunks, that's one wasted crypto+DOM serialization per chunk. Consider determining the boundary with a cheaper length estimate (e.g. node text length or a rough size projection) before doing the full serialization, or otherwise avoid discarding an already-serialized block. Low impact, but worth considering since serialization here is non-trivial.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/document-html-chunk.ts, line 41:

<comment>Every chunk read runs a full, expensive serialization (SHA-256 fingerprint + DOMSerializer pass) on the boundary block only to discover it won't fit, then discards that work and `break`s. For a large document read in many chunks, that's one wasted crypto+DOM serialization per chunk. Consider determining the boundary with a cheaper length estimate (e.g. node text length or a rough size projection) before doing the full serialization, or otherwise avoid discarding an already-serialized block. Low impact, but worth considering since serialization here is non-trivial.</comment>

<file context>
@@ -0,0 +1,64 @@
+	let characters = 0;
+	let endOffset = offset;
+	while (endOffset < document.childCount) {
+		const block = await serializeTiptapNodeToAiHtml(document.child(endOffset));
+		const separatorCharacters = content.length > 0 ? 1 : 0;
+		if (
</file context>

[cubic-dev-ai]

P3: Clicking a citation that points at a source that no longer exists (or isn't in the current workspace) silently does nothing: the handler ignores reveal()'s boolean return and always prevents the default. Since citations are meant to read as prose even after a source is deleted, guard the navigation with hasItem() (added to the location context in this same PR) so a stale citation doesn't present a dead, no-feedback click, or else provide a fallback message when reveal fails.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/document-editor/DocumentEditorSurface.tsx, line 103:

<comment>Clicking a citation that points at a source that no longer exists (or isn't in the current workspace) silently does nothing: the handler ignores reveal()'s boolean return and always prevents the default. Since citations are meant to read as prose even after a source is deleted, guard the navigation with hasItem() (added to the location context in this same PR) so a stale citation doesn't present a dead, no-feedback click, or else provide a fallback message when reveal fails.</comment>

<file context>
@@ -85,6 +89,26 @@ function DocumentEditorInstance({
+					}
+
+					const page = Number(citation?.getAttribute("data-page"));
+					event.preventDefault();
+					reveal(
+						Number.isInteger(page) && page > 0
</file context>

[cubic-dev-ai]

P3: The client-side action type on AiChatToolChildActivity omits lineChanges, even though the server's orchestration contract includes it (optionally) and the codemode group path in getAiChatDocumentEditGroups reads action.lineChanges to compute the Lines: +N −M tally shown in the receipt. Right now it works only because the server happens to attach the field and the consumer's parameter type leaves it optional; the declared source interface doesn't reflect the actual payload shape. Aligning the type with the server contract (and the consumer) would make the line-tally path type-truthful and less fragile to future refactors or stricter typing.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/ai-chat/ai-chat-codemode-activity.ts, line 16:

<comment>The client-side `action` type on `AiChatToolChildActivity` omits `lineChanges`, even though the server's orchestration contract includes it (optionally) and the codemode group path in `getAiChatDocumentEditGroups` reads `action.lineChanges` to compute the Lines: +N −M tally shown in the receipt. Right now it works only because the server happens to attach the field and the consumer's parameter type leaves it optional; the declared source interface doesn't reflect the actual payload shape. Aligning the type with the server contract (and the consumer) would make the line-tally path type-truthful and less fragile to future refactors or stricter typing.</comment>

<file context>
@@ -13,6 +13,12 @@ import {
 } from "#/features/workspaces/components/ai-chat/ai-chat-tool-receipts";
 
 export interface AiChatToolChildActivity {
+	action?: {
+		kind: "document-edit";
+		itemId: string;
</file context>

Suggested change

	action?: {
	kind: "document-edit";
	itemId: string;
	path: string;
	receiptId: string;
	action?: {
	kind: "document-edit";
	itemId: string;
	lineChanges?: { added: number; removed: number };
	path: string;
	receiptId: string;
	};

[capy-ai]

Capy auto-review is paused for this organization because the usage-cycle auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews.

[cubic-dev-ai]

10 issues found and verified against the latest diff

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/features/workspaces/documents/document-edit-review-queries.ts">

<violation number="1" location="src/features/workspaces/documents/document-edit-review-queries.ts:17">
P2: Different valid receipt lists can share this query key, allowing one transcript row to reuse another row's review verdict and show the wrong diff or status. Using `target.receiptIds` directly as the query-key segment preserves the array boundaries.</violation>

<violation number="2" location="src/features/workspaces/documents/document-edit-review-queries.ts:24">
P2: Reopening a receipt can briefly show the old AI diff after the user has edited or undone the document: `staleTime: 0` still serves the retained cached `ready` result while refetching. Setting `gcTime: 0` (or explicitly removing the query when review closes) would prevent a prior server verdict from being reused.</violation>
</file>

<file name="src/features/workspaces/documents/use-document-edit-receipt-undo.ts">

<violation number="1" location="src/features/workspaces/documents/use-document-edit-receipt-undo.ts:22">
P2: Undoing one receipt can close a different document's review if the user opens it before the request finishes. Making the completion path target-aware (or making `hideReview` accept and verify the receipt target) would prevent an older mutation from clearing newer review state.</violation>

<violation number="2" location="src/features/workspaces/documents/use-document-edit-receipt-undo.ts:24">
P2: A repeated undo is shown as “Changes undone.” even though it changed nothing. The server result needs distinct success and already-undone statuses so this hook can display the correct outcome.</violation>
</file>

<file name="src/features/workspaces/content/workspace-content-reader.ts">

<violation number="1" location="src/features/workspaces/content/workspace-content-reader.ts:80">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

Once `readBudgetExhausted` is set, subsequent items are immediately rejected without invoking `readWorkspaceItem`. This is a meaningful behavior change—previously each item was fully read and could surface its own specific failure before being budget-rejected. The existing batch-budget test (`"bounds total content returned by a batch"`) verifies that later items receive `read_budget_exceeded`, but it does not assert that their reads are actually skipped. A regression assertion (e.g., spying on `getDocumentSession` or `readHtmlChunk`) should be added to exercise and protect this new early-exit behavior.</violation>

<violation number="2" location="src/features/workspaces/content/workspace-content-reader.ts:107">
P3: The reader now fires `listItemRelations(...)` for each ready item during the main loop and only awaits them later in `attachRelationPaths` (via Promise.all). If any subsequent iteration throws a non-WorkspacePageSelectionError (for example a malformed continuation cursor that fails `decodeWorkspaceContentCursor`), the catch rethrows before `attachRelationPaths` runs, so the already-started relations promises are abandoned. If one of those rejects it becomes an unhandled promise rejection that can crash the worker under Node's default unhandled-rejection handling. Consider settling (or attaching a no-op catch to) the relations promises so an early throw can't leave dangling rejections.</violation>
</file>

<file name="src/features/workspaces/operations/edit-item.ts">

<violation number="1" location="src/features/workspaces/operations/edit-item.ts:83">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The new orchestration logic in `editWorkspaceItemOperation`—including citation resolution into HTML edits, `operationId` forwarding for conflict detection, and `lineChanges`/`itemId` propagation for review receipts—is not exercised by any tests. For a behavior-change PR of this scope, adding unit or integration tests for this operation handler would help prevent regressions and validate that the new integration behaves as intended.</violation>
</file>

<file name="src/features/workspaces/components/document-editor/document-editor-toolbar-actions.tsx">

<violation number="1" location="src/features/workspaces/components/document-editor/document-editor-toolbar-actions.tsx:80">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

This behavior change introduces a new Heading 4 toolbar action and renames the active-state model from fontSize to textStyle, but no tests exercise the new isTextStyle logic or the heading4 command. Since a regression-style assertion here is practical (e.g., verifying active returns true for the matching text style and false for others), the changed behavior should be covered by tests before shipping. Consider adding unit tests for documentTextStyleActions and the isTextStyle active-state helper.</violation>
</file>

<file name="src/features/workspaces/ai/workspace-tool-result-adapters.ts">

<violation number="1" location="src/features/workspaces/ai/workspace-tool-result-adapters.ts:54">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The `workspace_create_items` result adapter no longer passes through `warnings`, but this behavior change is not documented in the PR description and is not covered by any test. Because the Zod schema no longer includes `warnings`, the adapter silently strips any warnings that are still present in the raw operation result. A regression-style test asserting the new receipt shape (or confirming warnings are intentionally dropped) should be added.</violation>
</file>

<file name="src/features/workspaces/components/document-editor/DocumentToolbar.tsx">

<violation number="1" location="src/features/workspaces/components/document-editor/DocumentToolbar.tsx:66">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The comment says the read-only toolbar "shows only what still works," but the only UI it renders (`DocumentMoreMenu` / `DocumentExportMenuGroup`) contains exclusively disabled placeholder items labeled "Soon" (PDF and Google Docs). Nothing in the toolbar is actually functional for read-only users, so the comment makes a claim about behavior the code does not implement. Consider rephrasing or removing the misleading rationale.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: Different valid receipt lists can share this query key, allowing one transcript row to reuse another row's review verdict and show the wrong diff or status. Using target.receiptIds directly as the query-key segment preserves the array boundaries.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/document-edit-review-queries.ts, line 17:

<comment>Different valid receipt lists can share this query key, allowing one transcript row to reuse another row's review verdict and show the wrong diff or status. Using `target.receiptIds` directly as the query-key segment preserves the array boundaries.</comment>

<file context>
@@ -0,0 +1,26 @@
+			"workspace-document-edit-review",
+			target.workspaceId,
+			target.itemId,
+			target.receiptIds.join(":"),
+		],
+		queryFn: () => getDocumentEditReceiptReviewFn({ data: target }),
</file context>

[cubic-dev-ai]

P2: Reopening a receipt can briefly show the old AI diff after the user has edited or undone the document: staleTime: 0 still serves the retained cached ready result while refetching. Setting gcTime: 0 (or explicitly removing the query when review closes) would prevent a prior server verdict from being reused.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/document-edit-review-queries.ts, line 24:

<comment>Reopening a receipt can briefly show the old AI diff after the user has edited or undone the document: `staleTime: 0` still serves the retained cached `ready` result while refetching. Setting `gcTime: 0` (or explicitly removing the query when review closes) would prevent a prior server verdict from being reused.</comment>

<file context>
@@ -0,0 +1,26 @@
+		// whether the document still matches the receipt, and a verdict cached
+		// from an earlier open would mark the reader's own later writing as the
+		// assistant's.
+		staleTime: 0,
+	});
+}
</file context>

[cubic-dev-ai]

P2: A repeated undo is shown as “Changes undone.” even though it changed nothing. The server result needs distinct success and already-undone statuses so this hook can display the correct outcome.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/use-document-edit-receipt-undo.ts, line 24:

<comment>A repeated undo is shown as “Changes undone.” even though it changed nothing. The server result needs distinct success and already-undone statuses so this hook can display the correct outcome.</comment>

<file context>
@@ -0,0 +1,42 @@
+			// after that would be showing a diff we have just been told is wrong.
+			hideReview();
+
+			if (result.status === "reverted") {
+				toast.success("Changes undone.");
+			} else {
</file context>

[cubic-dev-ai]

P2: Undoing one receipt can close a different document's review if the user opens it before the request finishes. Making the completion path target-aware (or making hideReview accept and verify the receipt target) would prevent an older mutation from clearing newer review state.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/use-document-edit-receipt-undo.ts, line 22:

<comment>Undoing one receipt can close a different document's review if the user opens it before the request finishes. Making the completion path target-aware (or making `hideReview` accept and verify the receipt target) would prevent an older mutation from clearing newer review state.</comment>

<file context>
@@ -0,0 +1,42 @@
+			// Every outcome ends the review: it either just undid the changes, or
+			// told us they no longer describe the document. Leaving the marks up
+			// after that would be showing a diff we have just been told is wrong.
+			hideReview();
+
+			if (result.status === "reverted") {
</file context>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

The new orchestration logic in editWorkspaceItemOperation—including citation resolution into HTML edits, operationId forwarding for conflict detection, and lineChanges/itemId propagation for review receipts—is not exercised by any tests. For a behavior-change PR of this scope, adding unit or integration tests for this operation handler would help prevent regressions and validate that the new integration behaves as intended.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/operations/edit-item.ts, line 83:

<comment>The new orchestration logic in `editWorkspaceItemOperation`—including citation resolution into HTML edits, `operationId` forwarding for conflict detection, and `lineChanges`/`itemId` propagation for review receipts—is not exercised by any tests. For a behavior-change PR of this scope, adding unit or integration tests for this operation handler would help prevent regressions and validate that the new integration behaves as intended.</comment>

<file context>
@@ -77,15 +80,26 @@ export async function editWorkspaceItemOperation(
 
-	const result = await documentSession.applyMarkdownEdits({
-		edits,
+	const result = await documentSession.applyEdits({
+		edits: await Promise.all(
+			edits.map(async (edit) =>
</file context>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

This behavior change introduces a new Heading 4 toolbar action and renames the active-state model from fontSize to textStyle, but no tests exercise the new isTextStyle logic or the heading4 command. Since a regression-style assertion here is practical (e.g., verifying active returns true for the matching text style and false for others), the changed behavior should be covered by tests before shipping. Consider adding unit tests for documentTextStyleActions and the isTextStyle active-state helper.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/document-editor/document-editor-toolbar-actions.tsx, line 80:

<comment>This behavior change introduces a new Heading 4 toolbar action and renames the active-state model from fontSize to textStyle, but no tests exercise the new isTextStyle logic or the heading4 command. Since a regression-style assertion here is practical (e.g., verifying active returns true for the matching text style and false for others), the changed behavior should be covered by tests before shipping. Consider adding unit tests for documentTextStyleActions and the isTextStyle active-state helper.</comment>

<file context>
@@ -42,39 +43,47 @@ export interface DocumentToolbarAction {
 		run: (editor) => editor.chain().focus().setHeading({ level: 3 }).run(),
 	},
+	{
+		id: "text-style-heading4",
+		icon: <Heading4 />,
+		label: "Heading 4",
</file context>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

The workspace_create_items result adapter no longer passes through warnings, but this behavior change is not documented in the PR description and is not covered by any test. Because the Zod schema no longer includes warnings, the adapter silently strips any warnings that are still present in the raw operation result. A regression-style test asserting the new receipt shape (or confirming warnings are intentionally dropped) should be added.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/ai/workspace-tool-result-adapters.ts, line 54:

<comment>The `workspace_create_items` result adapter no longer passes through `warnings`, but this behavior change is not documented in the PR description and is not covered by any test. Because the Zod schema no longer includes `warnings`, the adapter silently strips any warnings that are still present in the raw operation result. A regression-style test asserting the new receipt shape (or confirming warnings are intentionally dropped) should be added.</comment>

<file context>
@@ -51,7 +51,6 @@ const workspaceCreateItemsResultAdapter = defineWorkspaceToolResultAdapter({
 				path: z.string(),
 				type: z.enum(["document", "folder"]),
-				warnings: z.array(z.string()).optional(),
 			}),
 		),
 		references: z.array(workspaceReferenceRecordSchema),
</file context>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

The comment says the read-only toolbar "shows only what still works," but the only UI it renders (DocumentMoreMenu / DocumentExportMenuGroup) contains exclusively disabled placeholder items labeled "Soon" (PDF and Google Docs). Nothing in the toolbar is actually functional for read-only users, so the comment makes a claim about behavior the code does not implement. Consider rephrasing or removing the misleading rationale.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/document-editor/DocumentToolbar.tsx, line 66:

<comment>The comment says the read-only toolbar "shows only what still works," but the only UI it renders (`DocumentMoreMenu` / `DocumentExportMenuGroup`) contains exclusively disabled placeholder items labeled "Soon" (PDF and Google Docs). Nothing in the toolbar is actually functional for read-only users, so the comment makes a claim about behavior the code does not implement. Consider rephrasing or removing the misleading rationale.</comment>

<file context>
@@ -33,9 +36,45 @@ import {
+		);
+	}
+
+	// Read-only viewers get every formatting control disabled, which reads as a
+	// broken toolbar. Show only what still works.
+	if (!canEdit) {
</file context>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

Once readBudgetExhausted is set, subsequent items are immediately rejected without invoking readWorkspaceItem. This is a meaningful behavior change—previously each item was fully read and could surface its own specific failure before being budget-rejected. The existing batch-budget test ("bounds total content returned by a batch") verifies that later items receive read_budget_exceeded, but it does not assert that their reads are actually skipped. A regression assertion (e.g., spying on getDocumentSession or readHtmlChunk) should be added to exercise and protect this new early-exit behavior.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/content/workspace-content-reader.ts, line 80:

<comment>Once `readBudgetExhausted` is set, subsequent items are immediately rejected without invoking `readWorkspaceItem`. This is a meaningful behavior change—previously each item was fully read and could surface its own specific failure before being budget-rejected. The existing batch-budget test (`"bounds total content returned by a batch"`) verifies that later items receive `read_budget_exceeded`, but it does not assert that their reads are actually skipped. A regression assertion (e.g., spying on `getDocumentSession` or `readHtmlChunk`) should be added to exercise and protect this new early-exit behavior.</comment>

<file context>
@@ -72,6 +71,16 @@ export async function readWorkspaceContent(input: {
+			status: "failed" as const,
+			...(resolution.item.type === "file" ? { type: "file" as const } : {}),
+		};
+		if (readBudgetExhausted) {
+			results.push(readBudgetFailure);
+			continue;
</file context>

[cubic-dev-ai]

P3: The reader now fires listItemRelations(...) for each ready item during the main loop and only awaits them later in attachRelationPaths (via Promise.all). If any subsequent iteration throws a non-WorkspacePageSelectionError (for example a malformed continuation cursor that fails decodeWorkspaceContentCursor), the catch rethrows before attachRelationPaths runs, so the already-started relations promises are abandoned. If one of those rejects it becomes an unhandled promise rejection that can crash the worker under Node's default unhandled-rejection handling. Consider settling (or attaching a no-op catch to) the relations promises so an early throw can't leave dangling rejections.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/content/workspace-content-reader.ts, line 107:

<comment>The reader now fires `listItemRelations(...)` for each ready item during the main loop and only awaits them later in `attachRelationPaths` (via Promise.all). If any subsequent iteration throws a non-WorkspacePageSelectionError (for example a malformed continuation cursor that fails `decodeWorkspaceContentCursor`), the catch rethrows before `attachRelationPaths` runs, so the already-started relations promises are abandoned. If one of those rejects it becomes an unhandled promise rejection that can crash the worker under Node's default unhandled-rejection handling. Consider settling (or attaching a no-op catch to) the relations promises so an early throw can't leave dangling rejections.</comment>

<file context>
@@ -86,20 +95,16 @@ export async function readWorkspaceContent(input: {
 				item: resolution.item,
 				read,
-				relations: await input.kernel.listItemRelations({ itemId: resolution.item.id }),
+				relations: input.kernel.listItemRelations({ itemId: resolution.item.id }),
 			};
 			readyResults.push(pending);
</file context>

[cubic-dev-ai]

13 issues found and verified against the latest diff

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/features/workspaces/components/document-editor/DocumentEditorSurface.tsx">

<violation number="1" location="src/features/workspaces/components/document-editor/DocumentEditorSurface.tsx:104">
P2: Clicking a document citation whose source was deleted silently does nothing because this handler discards `reveal`'s failure result. Handling the `false` result and showing the existing “This source is no longer available.” toast would give document citations the same feedback as chat citations.</violation>
</file>

<file name="src/features/workspaces/components/ai-chat/AiChatDocumentEditActions.tsx">

<violation number="1" location="src/features/workspaces/components/ai-chat/AiChatDocumentEditActions.tsx:57">
P2: Receipt rows remain a static “Edited”/line-count display and never show the required Edited since/Undone status, so users cannot tell from the transcript that a receipt is no longer reviewable without clicking each row. Deriving and rendering the latest receipt status for each row would keep the persistent transcript accurate after undo or subsequent edits.</violation>
</file>

<file name="src/features/workspaces/components/ai-chat/ai-chat-document-edit-actions.ts">

<violation number="1" location="src/features/workspaces/components/ai-chat/ai-chat-document-edit-actions.ts:29">
P2: Edits from messages using the legacy sibling-tool representation disappear from the document-change receipt. When `action` is absent here, the compatibility path has already removed the original `workspace_edit_item` part, so preserving or deriving its document-edit action in that path would keep historical review and undo available.</violation>
</file>

<file name="src/features/workspaces/ai/ai-thread.ts">

<violation number="1" location="src/features/workspaces/ai/ai-thread.ts:175">
P2: Documents created or edited through `orchestrate` lose workspace citation links because its nested workspace tools do not receive the new resolver. Passing the same resolver through the turn-tool configuration and Code Mode tool catalog preserves citations for both direct and orchestrated operations.</violation>

<violation number="2" location="src/features/workspaces/ai/ai-thread.ts:529">
P2: A new document can persist a stale citation from an earlier chat turn because this resolver searches all transcript records even though workspace refs are turn-scoped. Limiting lookup to `activeWorkspaceReferences` keeps old refs unresolved instead of attaching outdated source metadata.</violation>
</file>

<file name="src/features/workspaces/documents/document-html-chunk.ts">

<violation number="1" location="src/features/workspaces/documents/document-html-chunk.ts:33">
P2: A fresh start read (offset 0) of an empty document now fails instead of returning an empty/tractable result. For a document with zero blocks, `document.childCount` is 0 and the guard `offset >= document.childCount` is true for `offset: 0`, so `readDocumentHtmlChunk` returns `undefined`. In `document-session.readHtmlChunk` that becomes `{ status: "invalid_offset" }`, and the reader maps a *start* mode read (which carries no cursor at all) to `{ code: "invalid_cursor", status: "failed" }`. The AI therefore cannot read an empty document it just created — workspace_read_items reports an invalid cursor on a legitimate first read, which is misleading and blocks create-then-edit workflows on empty documents. The test case "rejects a nonzero continuation offset for an empty document" implies offset 0 should be a valid start read, but no path produces a ready result for it.</violation>
</file>

<file name="src/features/workspaces/documents/document-ai-edits.ts">

<violation number="1" location="src/features/workspaces/documents/document-ai-edits.ts:181">
P3: Line tallies for atom blocks (rules, formulas, etc.) can be miscounted across whole-document rewrites. `countDocumentLines` keys atom lines by the full node JSON, which includes the internal `aiRef` attribute that `ensureProseMirrorDocumentAiRefs` regenerates on every top-level block during `replace_all`. As a result an atom whose visible content did not change is still reported as one line removed and one line added in the receipt tally. Consider stripping the ref (e.g. `withTiptapNodeAiRef(node, null)` before `toJSON()`, or key on the block's content) so the tally reflects visible content only, matching how text blocks are keyed by `textContent`.</violation>
</file>

<file name="src/features/workspaces/documents/tiptap-schema.ts">

<violation number="1" location="src/features/workspaces/documents/tiptap-schema.ts:121">
P3: This schema is shared with the live client editor (`getTiptapDocumentSchemaExtensions` is spread into the editor via `tiptap-extensions.ts`), so flipping `Link` `openOnClick` from `false` to `true` changes link interaction for every document being edited, not just the server-side HTML projection used for AI editing. Clicking a link in the editor now navigates to the URL instead of keeping the cursor/selection, a user-facing behavior change in normal editing. `openOnClick` is an editor-interaction option with no effect on server parse/serialize, so unless this editor UX change is intentional for the PR it reads as an incidental side effect of the schema refactor.</violation>
</file>

<file name="src/features/workspaces/documents/document-session.ts">

<violation number="1" location="src/features/workspaces/documents/document-session.ts:444">
P2: The undo/edit write path changed how the shared document fragment is replaced. Previously `replaceCurrentDocument` explicitly cleared the Y.XmlFragment (`fragment.delete(0, fragment.length)`) before calling `prosemirrorJSONToYXmlFragment`; the new `reconcileCurrentDocument` calls it directly on the existing fragment with no clear. If `prosemirrorJSONToYXmlFragment` merges into rather than replaces the fragment content, every applied edit, undo, and ref-reconciliation in `applyEdits`/`undoDocumentEditReceipt`/`getReferencedDocumentSnapshot` would append the full document again, compounding content. Please confirm the helper replaces its target fragment; if it does not, restore the `fragment.delete(0, fragment.length)` (and the surrounding transaction) before inserting.</violation>
</file>

<file name="src/features/workspaces/operations/document-citations.ts">

<violation number="1" location="src/features/workspaces/operations/document-citations.ts:14">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The new `resolveDocumentCitations` operation is introduced without unit tests, even though regression-style assertions are practical. The function has two distinct branches—no-op when refs are empty or the resolver is absent, and mapping resolved records back into HTML—that could be trivially covered with a mocked `WorkspaceAccessContext`. In a behavior-change PR, introducing untested core logic increases the risk of silent regressions in citation resolution. Adding a focused test file (e.g., `document-citations.test.ts`) would exercise both branches and document the expected behavior for future changes.</violation>
</file>

<file name="src/features/workspaces/documents/use-document-collaboration-session.ts">

<violation number="1" location="src/features/workspaces/documents/use-document-collaboration-session.ts:209">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The new local-persistence readiness condition that checks `session.ydoc.getXmlFragment(tiptapDocumentYjsField).length > 0` before marking a cached session ready is a behavior change with no test coverage. A practical regression assertion exists (seed local persistence with the `server-synced` flag and an empty tiptap fragment, assert the session is not marked ready), yet no tests cover this path. For a behavior-change PR, new gating logic should be validated by a regression test.</violation>
</file>

<file name="src/features/workspaces/components/document-editor/DocumentToolbar.tsx">

<violation number="1" location="src/features/workspaces/components/document-editor/DocumentToolbar.tsx:55">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

This behavior-change PR introduces new conditional render paths in DocumentToolbar (review mode, read-only mode, and a font-size→text-style rename) that are not exercised by any visible tests. Rule 1 flags behavior-change PRs where changed behavior is not tested, especially when regression-style assertions are practical. Consider adding component-level tests that verify the review controls render when activeReview.itemId matches, the minimal toolbar renders when canEdit is false, and the renamed text-style actions and icons behave correctly.</violation>
</file>

<file name="src/features/workspaces/ai/ai-thread-orchestration.ts">

<violation number="1" location="src/features/workspaces/ai/ai-thread-orchestration.ts:133">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The new behavior branches for model output transformation and edit-receipt metadata attachment are not covered by regression-style tests. `attachDocumentEditReceiptMetadata` has no test references at all in the test suite, and while `getAIThreadOrchestrationModelOutput` is tested as a standalone function, the `toModelOutput` property added to the tool returned by `createAIThreadOrchestrationTool` is never exercised. Since the existing worker test already mocks `createExecuteRuntime` and exercises connector tools, practical assertions could be added for both branches: one verifying that `workspace_edit_item` results carry the edit-receipt metadata, and another verifying the tool's `toModelOutput` produces the expected model-visible JSON shape.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: Clicking a document citation whose source was deleted silently does nothing because this handler discards reveal's failure result. Handling the false result and showing the existing “This source is no longer available.” toast would give document citations the same feedback as chat citations.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/document-editor/DocumentEditorSurface.tsx, line 104:

<comment>Clicking a document citation whose source was deleted silently does nothing because this handler discards `reveal`'s failure result. Handling the `false` result and showing the existing “This source is no longer available.” toast would give document citations the same feedback as chat citations.</comment>

<file context>
@@ -85,6 +89,26 @@ function DocumentEditorInstance({
+
+					const page = Number(citation?.getAttribute("data-page"));
+					event.preventDefault();
+					reveal(
+						Number.isInteger(page) && page > 0
+							? { itemId, kind: "pdf-page", pageNumber: page, version: 1 }
</file context>

[cubic-dev-ai]

P2: Receipt rows remain a static “Edited”/line-count display and never show the required Edited since/Undone status, so users cannot tell from the transcript that a receipt is no longer reviewable without clicking each row. Deriving and rendering the latest receipt status for each row would keep the persistent transcript accurate after undo or subsequent edits.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/ai-chat/AiChatDocumentEditActions.tsx, line 57:

<comment>Receipt rows remain a static “Edited”/line-count display and never show the required Edited since/Undone status, so users cannot tell from the transcript that a receipt is no longer reviewable without clicking each row. Deriving and rendering the latest receipt status for each row would keep the persistent transcript accurate after undo or subsequent edits.</comment>

<file context>
@@ -0,0 +1,107 @@
+	// document or switching tabs as needed. If they are no longer reviewable the
+	// document still opens and the overlay says why.
+	return (
+		<button
+			type="button"
+			className="flex w-full min-w-0 items-center gap-2 px-2.5 py-2 text-left transition-colors hover:bg-foreground/5"
</file context>

[cubic-dev-ai]

P2: Edits from messages using the legacy sibling-tool representation disappear from the document-change receipt. When action is absent here, the compatibility path has already removed the original workspace_edit_item part, so preserving or deriving its document-edit action in that path would keep historical review and undo available.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/ai-chat/ai-chat-document-edit-actions.ts, line 29:

<comment>Edits from messages using the legacy sibling-tool representation disappear from the document-change receipt. When `action` is absent here, the compatibility path has already removed the original `workspace_edit_item` part, so preserving or deriving its document-edit action in that path would keep historical review and undo available.</comment>

<file context>
@@ -0,0 +1,100 @@
+	for (const part of parts) {
+		if (isAiChatToolGroupPart(part)) {
+			for (const child of part.children) {
+				if (child.status === "completed" && child.action?.kind === "document-edit") {
+					addToGroup(groupsByItemId, seenReceiptIds, child.action);
+				}
</file context>

[cubic-dev-ai]

P2: Documents created or edited through orchestrate lose workspace citation links because its nested workspace tools do not receive the new resolver. Passing the same resolver through the turn-tool configuration and Code Mode tool catalog preserves citations for both direct and orchestrated operations.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/ai/ai-thread.ts, line 175:

<comment>Documents created or edited through `orchestrate` lose workspace citation links because its nested workspace tools do not receive the new resolver. Passing the same resolver through the turn-tool configuration and Code Mode tool catalog preserves citations for both direct and orchestrated operations.</comment>

<file context>
@@ -172,6 +172,7 @@ export function createAIThreadClass(getUserAIStore: () => typeof UserAIStore) {
 				onWorkspaceReferences: (records) => {
 					this._recordWorkspaceReferences(records);
 				},
+				resolveWorkspaceReferences: (refs) => this._resolveWorkspaceReferences(refs),
 			});
 		}
</file context>

[cubic-dev-ai]

P2: A new document can persist a stale citation from an earlier chat turn because this resolver searches all transcript records even though workspace refs are turn-scoped. Limiting lookup to activeWorkspaceReferences keeps old refs unresolved instead of attaching outdated source metadata.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/ai/ai-thread.ts, line 529:

<comment>A new document can persist a stale citation from an earlier chat turn because this resolver searches all transcript records even though workspace refs are turn-scoped. Limiting lookup to `activeWorkspaceReferences` keeps old refs unresolved instead of attaching outdated source metadata.</comment>

<file context>
@@ -517,6 +518,21 @@ export function createAIThreadClass(getUserAIStore: () => typeof UserAIStore) {
+		private async _resolveWorkspaceReferences(refs: readonly string[]) {
+			const wanted = new Set(refs);
+			const records = [
+				...collectWorkspaceReferenceRecords(await this.getMessages()),
+				...this.activeWorkspaceReferences,
+			];
</file context>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

The new local-persistence readiness condition that checks session.ydoc.getXmlFragment(tiptapDocumentYjsField).length > 0 before marking a cached session ready is a behavior change with no test coverage. A practical regression assertion exists (seed local persistence with the server-synced flag and an empty tiptap fragment, assert the session is not marked ready), yet no tests cover this path. For a behavior-change PR, new gating logic should be validated by a regression test.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/use-document-collaboration-session.ts, line 209:

<comment>The new local-persistence readiness condition that checks `session.ydoc.getXmlFragment(tiptapDocumentYjsField).length > 0` before marking a cached session ready is a behavior change with no test coverage. A practical regression assertion exists (seed local persistence with the `server-synced` flag and an empty tiptap fragment, assert the session is not marked ready), yet no tests cover this path. For a behavior-change PR, new gating logic should be validated by a regression test.</comment>

<file context>
@@ -203,7 +204,10 @@ function createActiveDocumentSession(input: {
-			if (wasServerSynced === localDocumentReadyValue) {
+			if (
+				wasServerSynced === localDocumentReadyValue &&
+				session.ydoc.getXmlFragment(tiptapDocumentYjsField).length > 0
+			) {
 				markReady();
</file context>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

This behavior-change PR introduces new conditional render paths in DocumentToolbar (review mode, read-only mode, and a font-size→text-style rename) that are not exercised by any visible tests. Rule 1 flags behavior-change PRs where changed behavior is not tested, especially when regression-style assertions are practical. Consider adding component-level tests that verify the review controls render when activeReview.itemId matches, the minimal toolbar renders when canEdit is false, and the renamed text-style actions and icons behave correctly.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/document-editor/DocumentToolbar.tsx, line 55:

<comment>This behavior-change PR introduces new conditional render paths in DocumentToolbar (review mode, read-only mode, and a font-size→text-style rename) that are not exercised by any visible tests. Rule 1 flags behavior-change PRs where changed behavior is not tested, especially when regression-style assertions are practical. Consider adding component-level tests that verify the review controls render when activeReview.itemId matches, the minimal toolbar renders when canEdit is false, and the renamed text-style actions and icons behave correctly.</comment>

<file context>
@@ -33,9 +36,45 @@ import {
+
+	// Reviewing borrows the toolbar rather than floating over the page: the
+	// formatting controls are unusable mid-review anyway, so the space is free.
+	if (activeReview?.itemId === itemId) {
+		return (
+			<DocumentEditReviewControls
</file context>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

The new behavior branches for model output transformation and edit-receipt metadata attachment are not covered by regression-style tests. attachDocumentEditReceiptMetadata has no test references at all in the test suite, and while getAIThreadOrchestrationModelOutput is tested as a standalone function, the toModelOutput property added to the tool returned by createAIThreadOrchestrationTool is never exercised. Since the existing worker test already mocks createExecuteRuntime and exercises connector tools, practical assertions could be added for both branches: one verifying that workspace_edit_item results carry the edit-receipt metadata, and another verifying the tool's toModelOutput produces the expected model-visible JSON shape.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/ai/ai-thread-orchestration.ts, line 133:

<comment>The new behavior branches for model output transformation and edit-receipt metadata attachment are not covered by regression-style tests. `attachDocumentEditReceiptMetadata` has no test references at all in the test suite, and while `getAIThreadOrchestrationModelOutput` is tested as a standalone function, the `toModelOutput` property added to the tool returned by `createAIThreadOrchestrationTool` is never exercised. Since the existing worker test already mocks `createExecuteRuntime` and exercises connector tools, practical assertions could be added for both branches: one verifying that `workspace_edit_item` results carry the edit-receipt metadata, and another verifying the tool's `toModelOutput` produces the expected model-visible JSON shape.</comment>

<file context>
@@ -114,11 +123,16 @@ class AIThreadToolSetConnector extends CodemodeConnector {
 								source: "codemode",
 							});
+
+							return toolName === "workspace_edit_item"
+								? attachDocumentEditReceiptMetadata(output, invocationId)
+								: output;
</file context>

[cubic-dev-ai]

P3: Line tallies for atom blocks (rules, formulas, etc.) can be miscounted across whole-document rewrites. countDocumentLines keys atom lines by the full node JSON, which includes the internal aiRef attribute that ensureProseMirrorDocumentAiRefs regenerates on every top-level block during replace_all. As a result an atom whose visible content did not change is still reported as one line removed and one line added in the receipt tally. Consider stripping the ref (e.g. withTiptapNodeAiRef(node, null) before toJSON(), or key on the block's content) so the tally reflects visible content only, matching how text blocks are keyed by textContent.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/document-ai-edits.ts, line 181:

<comment>Line tallies for atom blocks (rules, formulas, etc.) can be miscounted across whole-document rewrites. `countDocumentLines` keys atom lines by the full node JSON, which includes the internal `aiRef` attribute that `ensureProseMirrorDocumentAiRefs` regenerates on every top-level block during `replace_all`. As a result an atom whose visible content did not change is still reported as one line removed and one line added in the receipt tally. Consider stripping the ref (e.g. `withTiptapNodeAiRef(node, null)` before `toJSON()`, or key on the block's content) so the tally reflects visible content only, matching how text blocks are keyed by `textContent`.</comment>

<file context>
@@ -0,0 +1,312 @@
+			}
+			// A rule or a formula holds no text but still occupies a line.
+			if (node.isAtom) {
+				countLine(JSON.stringify(node.toJSON()));
+				return false;
+			}
</file context>

[cubic-dev-ai]

P3: This schema is shared with the live client editor (getTiptapDocumentSchemaExtensions is spread into the editor via tiptap-extensions.ts), so flipping Link openOnClick from false to true changes link interaction for every document being edited, not just the server-side HTML projection used for AI editing. Clicking a link in the editor now navigates to the URL instead of keeping the cursor/selection, a user-facing behavior change in normal editing. openOnClick is an editor-interaction option with no effect on server parse/serialize, so unless this editor UX change is intentional for the PR it reads as an incidental side effect of the schema refactor.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/tiptap-schema.ts, line 121:

<comment>This schema is shared with the live client editor (`getTiptapDocumentSchemaExtensions` is spread into the editor via `tiptap-extensions.ts`), so flipping `Link` `openOnClick` from `false` to `true` changes link interaction for every document being edited, not just the server-side HTML projection used for AI editing. Clicking a link in the editor now navigates to the URL instead of keeping the cursor/selection, a user-facing behavior change in normal editing. `openOnClick` is an editor-interaction option with no effect on server parse/serialize, so unless this editor UX change is intentional for the PR it reads as an incidental side effect of the schema refactor.</comment>

<file context>
@@ -40,7 +118,7 @@ export function getTiptapDocumentSchemaExtensions({
 		Highlight,
 		Link.configure({
-			openOnClick: false,
+			openOnClick: true,
 			autolink: true,
 			defaultProtocol: "https",
</file context>

[cubic-dev-ai]

10 issues found and verified against the latest diff

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/features/workspaces/components/document-editor/DocumentEditorSurface.tsx">

<violation number="1" location="src/features/workspaces/components/document-editor/DocumentEditorSurface.tsx:104">
P2: Clicking an inline citation whose source was deleted or is otherwise unavailable silently does nothing. Handle the `reveal` return value and surface the same unavailable-source feedback used by chat citations.</violation>
</file>

<file name="src/features/workspaces/components/ai-chat/ai-chat-document-edit-actions.ts">

<violation number="1" location="src/features/workspaces/components/ai-chat/ai-chat-document-edit-actions.ts:27">
P2: Document-edit receipts disappear for legacy assistant messages that contain a Code Mode part plus a sibling `workspace_edit_item`: legacy display grouping drops the sibling's document-edit action, and this branch skips the original part. Preserving the action when building legacy children, or retaining/processing those sibling tool parts here, keeps review and undo available for those messages.</violation>
</file>

<file name="src/features/workspaces/components/ai-chat/AiChatDocumentEditActions.tsx">

<violation number="1" location="src/features/workspaces/components/ai-chat/AiChatDocumentEditActions.tsx:61">
P2: Responses with more than eight successful edits to one document cannot be reviewed or undone, even though the latest document is valid. Pass only a supported contiguous receipt suffix or change the grouping/retention contract so evicted IDs are not sent.</violation>

<violation number="2" location="src/features/workspaces/components/ai-chat/AiChatDocumentEditActions.tsx:70">
P2: After Undo, the transcript receipt still looks like an active edit and offers no `Undone` or `Edited since` state. Add live receipt-status state or update the row when review and undo complete.</violation>
</file>

<file name="src/features/workspaces/ai/ai-thread-orchestration-contract.ts">

<violation number="1" location="src/features/workspaces/ai/ai-thread-orchestration-contract.ts:147">
P2: The Code Mode model can read the document-edit receipt ID even though this projection is intended to keep UI metadata app-only. Stripping only the final result is too late; the connector should remove UI metadata before returning the nested tool result or carry the receipt through a non-model-visible side channel.</violation>
</file>

<file name="src/features/workspaces/documents/document-ai-html.ts">

<violation number="1" location="src/features/workspaces/documents/document-ai-html.ts:262">
P2: AI-created documents can contain clickable citations to arbitrary items instead of only sources resolved from workspace refs. Reject or strip model-supplied `data-item-id`/`data-page` attributes and add durable metadata only for a successfully resolved citation ref.</violation>
</file>

<file name="src/features/workspaces/documents/tiptap-schema.ts">

<violation number="1" location="src/features/workspaces/documents/tiptap-schema.ts:121">
P2: This flips link behavior for the entire collaborative editor from `openOnClick: false` to `true`, so clicking any link in a document now opens it in a new tab rather than letting the user click to position the cursor and edit it. That is a cross-cutting change to every document edit surface, while the PR's feature is about the read-only review/undo workflow where clickable links make sense. If the intent was only to make links open in the review window, scope the behavior there; otherwise confirm this global editor change is intended.</violation>
</file>

<file name="src/features/workspaces/documents/use-document-edit-review-overlay.ts">

<violation number="1" location="src/features/workspaces/documents/use-document-edit-review-overlay.ts:46">
P1: If loading the review fails or the server reports the changes are no longer reviewable, the document editor is left permanently read-only. The effect freezes the editor with `setEditable(false)` up front, but the `isError` and `status !== "ready"` branches return early without scheduling any cleanup, and the only `setEditable(canEdit)` restore lives in the success-path cleanup. When `hideReview()` clears the active review, React re-runs the effect with `target === null`, and since no cleanup was registered the editor stays frozen. Recommend restoring editability on the failure paths (for example, call `editor.setEditable(canEdit)` before `hideReview()` in both early-return branches, or always return a cleanup that resets editability).</violation>
</file>

<file name="src/features/workspaces/operations/create-items.ts">

<violation number="1" location="src/features/workspaces/operations/create-items.ts:121">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

This behavior-change PR modifies `createWorkspaceItemsOperation` to resolve citations via `resolveDocumentCitations` and parse initial content with `parseDocumentAiHtml`, yet no tests exercise the new integration or the updated failure path. A regression-style test for document creation with HTML/citations—and for the `invalid_initial_content` rejection—should be added to guard against regressions.</violation>
</file>

<file name="src/features/workspaces/content/workspace-content-reader.ts">

<violation number="1" location="src/features/workspaces/content/workspace-content-reader.ts:50">
P1: Custom agent: **Flag AI Slop and Fabricated Changes**

The `readBudgetExhausted` flag changes batch read semantics so that once any item exceeds the byte budget, all later items are rejected without being read. Previously each item was checked independently, so a small item after a skipped large one could still be returned. This behavior change is not documented in the PR description or in code comments, and the existing test `bounds total content returned by a batch` cannot distinguish the new semantics from the old because every test item is identical. A regression-style assertion should be added (or the semantics should be reverted and documented) so that mixed-sized batches are handled correctly and items that would have returned real read errors are not masked as `read_budget_exceeded`.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P1: If loading the review fails or the server reports the changes are no longer reviewable, the document editor is left permanently read-only. The effect freezes the editor with setEditable(false) up front, but the isError and status !== "ready" branches return early without scheduling any cleanup, and the only setEditable(canEdit) restore lives in the success-path cleanup. When hideReview() clears the active review, React re-runs the effect with target === null, and since no cleanup was registered the editor stays frozen. Recommend restoring editability on the failure paths (for example, call editor.setEditable(canEdit) before hideReview() in both early-return branches, or always return a cleanup that resets editability).

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/use-document-edit-review-overlay.ts, line 45:

<comment>If loading the review fails or the server reports the changes are no longer reviewable, the document editor is left permanently read-only. The effect freezes the editor with `setEditable(false)` up front, but the `isError` and `status !== "ready"` branches return early without scheduling any cleanup, and the only `setEditable(canEdit)` restore lives in the success-path cleanup. When `hideReview()` clears the active review, React re-runs the effect with `target === null`, and since no cleanup was registered the editor stays frozen. Recommend restoring editability on the failure paths (for example, call `editor.setEditable(canEdit)` before `hideReview()` in both early-return branches, or always return a cleanup that resets editability).</comment>

<file context>
@@ -0,0 +1,94 @@
+		// Freeze on request rather than on arrival: the verdict takes a round trip,
+		// and a document that accepts typing in the meantime is one whose marks
+		// will not match it.
+		editor.setEditable(false);
+
+		if (reviewQuery.isError) {
</file context>

[cubic-dev-ai]

P1: Custom agent: Flag AI Slop and Fabricated Changes

The readBudgetExhausted flag changes batch read semantics so that once any item exceeds the byte budget, all later items are rejected without being read. Previously each item was checked independently, so a small item after a skipped large one could still be returned. This behavior change is not documented in the PR description or in code comments, and the existing test bounds total content returned by a batch cannot distinguish the new semantics from the old because every test item is identical. A regression-style assertion should be added (or the semantics should be reverted and documented) so that mixed-sized batches are handled correctly and items that would have returned real read errors are not masked as read_budget_exceeded.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/content/workspace-content-reader.ts, line 50:

<comment>The `readBudgetExhausted` flag changes batch read semantics so that once any item exceeds the byte budget, all later items are rejected without being read. Previously each item was checked independently, so a small item after a skipped large one could still be returned. This behavior change is not documented in the PR description or in code comments, and the existing test `bounds total content returned by a batch` cannot distinguish the new semantics from the old because every test item is identical. A regression-style assertion should be added (or the semantics should be reverted and documented) so that mixed-sized batches are handled correctly and items that would have returned real read errors are not masked as `read_budget_exceeded`.</comment>

<file context>
@@ -49,6 +47,7 @@ export async function readWorkspaceContent(input: {
 	const results: WorkspaceContentReadResult[] = [];
 	const readyResults: PendingReadyResult[] = [];
 	let returnedContentBytes = 0;
+	let readBudgetExhausted = false;
 
 	// Reads stay ordered so each body is consumed before the shared byte budget advances.
</file context>

[cubic-dev-ai]

P2: Clicking an inline citation whose source was deleted or is otherwise unavailable silently does nothing. Handle the reveal return value and surface the same unavailable-source feedback used by chat citations.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/document-editor/DocumentEditorSurface.tsx, line 104:

<comment>Clicking an inline citation whose source was deleted or is otherwise unavailable silently does nothing. Handle the `reveal` return value and surface the same unavailable-source feedback used by chat citations.</comment>

<file context>
@@ -85,6 +89,26 @@ function DocumentEditorInstance({
+
+					const page = Number(citation?.getAttribute("data-page"));
+					event.preventDefault();
+					reveal(
+						Number.isInteger(page) && page > 0
+							? { itemId, kind: "pdf-page", pageNumber: page, version: 1 }
</file context>

[cubic-dev-ai]

P2: Document-edit receipts disappear for legacy assistant messages that contain a Code Mode part plus a sibling workspace_edit_item: legacy display grouping drops the sibling's document-edit action, and this branch skips the original part. Preserving the action when building legacy children, or retaining/processing those sibling tool parts here, keeps review and undo available for those messages.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/ai-chat/ai-chat-document-edit-actions.ts, line 27:

<comment>Document-edit receipts disappear for legacy assistant messages that contain a Code Mode part plus a sibling `workspace_edit_item`: legacy display grouping drops the sibling's document-edit action, and this branch skips the original part. Preserving the action when building legacy children, or retaining/processing those sibling tool parts here, keeps review and undo available for those messages.</comment>

<file context>
@@ -0,0 +1,100 @@
+	const seenReceiptIds = new Set<string>();
+
+	for (const part of parts) {
+		if (isAiChatToolGroupPart(part)) {
+			for (const child of part.children) {
+				if (child.status === "completed" && child.action?.kind === "document-edit") {
</file context>

[cubic-dev-ai]

P2: After Undo, the transcript receipt still looks like an active edit and offers no Undone or Edited since state. Add live receipt-status state or update the row when review and undo complete.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/ai-chat/AiChatDocumentEditActions.tsx, line 70:

<comment>After Undo, the transcript receipt still looks like an active edit and offers no `Undone` or `Edited since` state. Add live receipt-status state or update the row when review and undo complete.</comment>

<file context>
@@ -0,0 +1,107 @@
+				<div className="truncate text-sm" title={group.path}>
+					<DocumentPathLabel path={group.path} />
+				</div>
+				<ChangeSummary lineChanges={group.lineChanges} />
+			</div>
+		</button>
</file context>

[cubic-dev-ai]

P2: Responses with more than eight successful edits to one document cannot be reviewed or undone, even though the latest document is valid. Pass only a supported contiguous receipt suffix or change the grouping/retention contract so evicted IDs are not sent.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/ai-chat/AiChatDocumentEditActions.tsx, line 61:

<comment>Responses with more than eight successful edits to one document cannot be reviewed or undone, even though the latest document is valid. Pass only a supported contiguous receipt suffix or change the grouping/retention contract so evicted IDs are not sent.</comment>

<file context>
@@ -0,0 +1,107 @@
+			type="button"
+			className="flex w-full min-w-0 items-center gap-2 px-2.5 py-2 text-left transition-colors hover:bg-foreground/5"
+			onClick={() => {
+				if (!showReview({ itemId: group.itemId, receiptIds: group.receiptIds })) {
+					toast.error("This document no longer exists.");
+				}
</file context>

[cubic-dev-ai]

P2: The Code Mode model can read the document-edit receipt ID even though this projection is intended to keep UI metadata app-only. Stripping only the final result is too late; the connector should remove UI metadata before returning the nested tool result or carry the receipt through a non-model-visible side channel.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/ai/ai-thread-orchestration-contract.ts, line 147:

<comment>The Code Mode model can read the document-edit receipt ID even though this projection is intended to keep UI metadata app-only. Stripping only the final result is too late; the connector should remove UI metadata before returning the nested tool result or carry the receipt through a non-model-visible side channel.</comment>

<file context>
@@ -130,7 +144,7 @@ export function normalizeAIThreadOrchestrationOutput(output: unknown): AIThreadO
 			status: parsed.data.status,
 			executionId: parsed.data.executionId,
-			result: parsed.data.result,
+			result: stripAIThreadToolUiMetadata(parsed.data.result),
 			calls,
 			outcome: childOutcome,
</file context>

[cubic-dev-ai]

P2: AI-created documents can contain clickable citations to arbitrary items instead of only sources resolved from workspace refs. Reject or strip model-supplied data-item-id/data-page attributes and add durable metadata only for a successfully resolved citation ref.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/document-ai-html.ts, line 262:

<comment>AI-created documents can contain clickable citations to arbitrary items instead of only sources resolved from workspace refs. Reject or strip model-supplied `data-item-id`/`data-page` attributes and add durable metadata only for a successfully resolved citation ref.</comment>

<file context>
@@ -0,0 +1,278 @@
+
+	for (const element of htmlDocument.body.querySelectorAll("citation[ref]")) {
+		const location = locationsByRef.get(element.getAttribute("ref") ?? "");
+		element.removeAttribute("ref");
+
+		if (!location) {
</file context>

[cubic-dev-ai]

P2: This flips link behavior for the entire collaborative editor from openOnClick: false to true, so clicking any link in a document now opens it in a new tab rather than letting the user click to position the cursor and edit it. That is a cross-cutting change to every document edit surface, while the PR's feature is about the read-only review/undo workflow where clickable links make sense. If the intent was only to make links open in the review window, scope the behavior there; otherwise confirm this global editor change is intended.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/tiptap-schema.ts, line 121:

<comment>This flips link behavior for the entire collaborative editor from `openOnClick: false` to `true`, so clicking any link in a document now opens it in a new tab rather than letting the user click to position the cursor and edit it. That is a cross-cutting change to every document edit surface, while the PR's feature is about the read-only review/undo workflow where clickable links make sense. If the intent was only to make links open in the review window, scope the behavior there; otherwise confirm this global editor change is intended.</comment>

<file context>
@@ -40,7 +118,7 @@ export function getTiptapDocumentSchemaExtensions({
 		Highlight,
 		Link.configure({
-			openOnClick: false,
+			openOnClick: true,
 			autolink: true,
 			defaultProtocol: "https",
</file context>

Suggested change

	openOnClick: true,
	openOnClick: false,

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

This behavior-change PR modifies createWorkspaceItemsOperation to resolve citations via resolveDocumentCitations and parse initial content with parseDocumentAiHtml, yet no tests exercise the new integration or the updated failure path. A regression-style test for document creation with HTML/citations—and for the invalid_initial_content rejection—should be added to guard against regressions.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/operations/create-items.ts, line 121:

<comment>This behavior-change PR modifies `createWorkspaceItemsOperation` to resolve citations via `resolveDocumentCitations` and parse initial content with `parseDocumentAiHtml`, yet no tests exercise the new integration or the updated failure path. A regression-style test for document creation with HTML/citations—and for the `invalid_initial_content` rejection—should be added to guard against regressions.</comment>

<file context>
@@ -118,7 +118,17 @@ export async function createWorkspaceItemsOperation(
 		}
 
-		const initialContent = getCreateWorkspaceItemInitialContent(itemInput);
+		const initialContent = getCreateWorkspaceItemInitialContent(
+			itemInput.type === "document" && itemInput.initialContent !== undefined
+				? {
</file context>

[cubic-dev-ai]

15 issues found across 59 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/features/workspaces/components/document-editor/document-editor-state.ts">

<violation number="1" location="src/features/workspaces/components/document-editor/document-editor-state.ts:169">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

This behavior change adds heading level 4 detection and renames the state union from `DocumentFontSize` to `DocumentTextStyle`, but no tests in the suite exercise `getActiveBlock` or the new `textStyle` values. A practical regression test (for example, mocking a TipTap editor and asserting the returned block for each heading level and the paragraph default) would prevent silent breakage of the toolbar state. Please add unit tests that cover the new `heading4` branch and the semantic rename.</violation>
</file>

<file name="src/features/workspaces/components/document-editor/document-editor-toolbar-actions.tsx">

<violation number="1" location="src/features/workspaces/components/document-editor/document-editor-toolbar-actions.tsx:80">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The toolbar semantics changed from font-size-based actions to text-style-based actions (paragraph and heading levels 1–4), including a new Heading 4 action with its own `active` state, `run` command, and icon mapping. However, there is no visible test coverage exercising this new behavior. Consider adding unit tests that assert the `isTextStyle` active-state logic, the `getTextStyleIcon` mappings, and the TipTap commands invoked by each `documentTextStyleActions` entry—including the new heading level 4—to prevent regressions in the document editor toolbar.</violation>
</file>

<file name="src/features/workspaces/documents/use-document-collaboration-session.ts">

<violation number="1" location="src/features/workspaces/documents/use-document-collaboration-session.ts:207">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The modified persistence-based readiness condition introduces a real behavior change, but there's no visible test coverage for it. A session that has the cached 'server-synced' marker in IndexedDB but an empty tiptap XML fragment will no longer be marked ready early and will instead wait for the websocket sync event. This edge case is now silently altered without regression protection. Consider adding a test that mocks the IndexedDB marker and toggles an empty vs. non-empty `Y.XmlFragment` to assert whether `markReady()` fires without a websocket sync — this directly exercises the change and guards against regressions.</violation>
</file>

<file name="src/features/workspaces/components/document-editor/DocumentEditorSurface.tsx">

<violation number="1" location="src/features/workspaces/components/document-editor/DocumentEditorSurface.tsx:92">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

This PR adds new user-visible citation-click navigation behavior, but no corresponding tests were found that exercise this interaction. Rule 1 (Flag AI Slop and Fabricated Changes) specifically calls out behavior-change PRs where changed behavior is not exercised by tests, especially when a regression-style assertion is practical. The click handler extracts `data-item-id` and `data-page`, parses the page conditionally, and calls `reveal()` — all of which is testable. Similarly, the newly wired `useDocumentEditReviewOverlay` hook lacks test coverage. Consider adding tests that verify the navigation logic and overlay integration to prevent regressions.</violation>

<violation number="2" location="src/features/workspaces/components/document-editor/DocumentEditorSurface.tsx:103">
P3: Clicking a citation that points at an item that has since been deleted or renamed silently does nothing. The new click handler always calls event.preventDefault() and returns true as if the citation was actioned, but it discards the boolean reveal() returns (workspace-location-context.tsx returns Boolean(viewInstanceId), which is false when the target item no longer exists). Because the new styles.css gives citations cursor:pointer, a stale citation still looks clickable but produces no navigation and no feedback. Consider checking reveal()'s result (or hasItem) and, when it returns false, falling back to opening the plain document item or otherwise signalling that the source is unavailable, rather than swallowing the click.</violation>
</file>

<file name="src/features/workspaces/ai/ai-thread-orchestration.ts">

<violation number="1" location="src/features/workspaces/ai/ai-thread-orchestration.ts:80">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

This behavior-change PR adds new orchestration wiring (`toModelOutput` on the orchestration tool and receipt-metadata attachment for `workspace_edit_item`) without tests that exercise these exact code paths. The related output-normalization helpers are tested, but the wiring that connects them into the tool lifecycle is not. A regression-style test should assert that `createAIThreadOrchestrationTool` returns a tool whose `toModelOutput` produces the expected compact projection, and that a `workspace_edit_item` invocation through the connector carries receipt metadata keyed by `invocationId`.</violation>
</file>

<file name="src/features/workspaces/documents/document-ai-html.test.ts">

<violation number="1" location="src/features/workspaces/documents/document-ai-html.test.ts:23">
P3: The round-trip test never verifies the round-trip: it only checks substrings and that re-parsing yields `type: "doc"`, so silently dropped or reordered blocks still pass despite the test name and the PR's "remains lossless" claim. Consider asserting the re-parsed document deep-equals the original (or a normalized snapshot) to actually guard the round-trip.</violation>
</file>

<file name="src/features/workspaces/ai/ai-thread.ts">

<violation number="1" location="src/features/workspaces/ai/ai-thread.ts:175">
P1: Document citations are lost when the assistant edits through `orchestrate`: the resolver is wired into direct workspace tools here, but not into the separately constructed Code Mode workspace tools. Thread the resolver through `createAIThreadTurnToolConfig` and its tool catalog as well so both execution paths resolve `<citation ref>` locations.</violation>
</file>

<file name="src/features/workspaces/operations/edit-item.ts">

<violation number="1" location="src/features/workspaces/operations/edit-item.ts:84">
P2: A batch with multiple citation-bearing HTML edits now rereads the chat transcript once per edit, creating up to 40 concurrent Durable Object reads for one tool call and avoidable latency/load. Resolving the union of citation refs once and reusing the records for each HTML fragment would preserve behavior without the per-edit read amplification.</violation>
</file>

<file name="src/features/workspaces/documents/use-document-edit-receipt-undo.ts">

<violation number="1" location="src/features/workspaces/documents/use-document-edit-receipt-undo.ts:30">
P2: A checkpoint failure after the undo commit leaves the document in undo state while the UI stays in the “Reviewing changes” mode. Closing or revalidating the review on mutation errors would prevent stale, read-only review state; the review can be reopened to retry when the failure occurred before the commit.</violation>
</file>

<file name="src/features/workspaces/operations/document-citations.ts">

<violation number="1" location="src/features/workspaces/operations/document-citations.ts:26">
P2: Model-supplied citation metadata can survive resolution: an unresolved ref can retain a fabricated `data-item-id`, and an item citation can retain a fabricated `data-page`. Strip server-owned citation attributes from ref-bearing tags before applying locations so only a resolved workspace record supplies them.</violation>

<violation number="2" location="src/features/workspaces/operations/document-citations.ts:28">
P2: A turn-local ref can resolve to the wrong workspace item when the same short ref appears in more than one transcript turn, because the map overwrites earlier records without checking their locations. Preserve only unambiguous refs (matching the collision handling used by `workspace-citations`) before writing the citation.</violation>
</file>

<file name="src/features/workspaces/documents/use-document-edit-review-overlay.ts">

<violation number="1" location="src/features/workspaces/documents/use-document-edit-review-overlay.ts:46">
P1: When a review fails to load or the server reports the receipt as unavailable (not_found, content_changed, not_latest, reverted, review_unavailable), the document editor is left permanently frozen in read-only mode. The effect calls `editor.setEditable(false)` up front, but the error and 'not ready' branches call `hideReview()` and `return` before any cleanup is registered; since `hideReview()` nulls the target, the effect re-runs and exits through the `if (!editor || !target) return;` guard without ever restoring `editor.setEditable(canEdit)`. The user is then stuck unable to edit that document until it is remounted. Consider restoring editability in those early-return branches (or always registering a cleanup that restores it) before calling hideReview().</violation>
</file>

<file name="src/features/workspaces/ai/ai-thread-orchestration-contract.ts">

<violation number="1" location="src/features/workspaces/ai/ai-thread-orchestration-contract.ts:208">
P3: The model-facing boundary strips app-only `action` from calls but does not strip the `__thinkexUi`/receipt metadata from `result` — it only stays clean because `execute` happens to feed it already-normalized (pre-stripped) output. If this exported helper is ever called with the raw runtime output (its signature accepts any `output`), the internal receipt ID would surface to the model. Applying `stripAIThreadToolUiMetadata` to `result` here (as `normalizeAIThreadOrchestrationOutput` already does) would make the boundary self-contained and consistent.</violation>
</file>

<file name="src/features/workspaces/documents/document-session.ts">

<violation number="1" location="src/features/workspaces/documents/document-session.ts:443">
P2: `reconcileCurrentDocument` now repopulates an existing Yjs `XmlFragment` without clearing it first, whereas the previous `replaceCurrentDocument` explicitly did `fragment.delete(0, fragment.length)` and wrapped the write in `this.document.transact(...)`. If `prosemirrorJSONToYXmlFragment` appends (as the old code's explicit delete strongly suggests), every AI edit and every ref reconciliation would duplicate the document's top-level blocks, and dropping the `transact` wrapper also removes the atomicity the old code guaranteed when streaming updates to connected clients. Please verify the library clears the fragment and restore the explicit clear/transaction (as in the removed code) if it does not.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P1: Document citations are lost when the assistant edits through orchestrate: the resolver is wired into direct workspace tools here, but not into the separately constructed Code Mode workspace tools. Thread the resolver through createAIThreadTurnToolConfig and its tool catalog as well so both execution paths resolve <citation ref> locations.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/ai/ai-thread.ts, line 175:

<comment>Document citations are lost when the assistant edits through `orchestrate`: the resolver is wired into direct workspace tools here, but not into the separately constructed Code Mode workspace tools. Thread the resolver through `createAIThreadTurnToolConfig` and its tool catalog as well so both execution paths resolve `<citation ref>` locations.</comment>

<file context>
@@ -172,6 +172,7 @@ export function createAIThreadClass(getUserAIStore: () => typeof UserAIStore) {
 				onWorkspaceReferences: (records) => {
 					this._recordWorkspaceReferences(records);
 				},
+				resolveWorkspaceReferences: (refs) => this._resolveWorkspaceReferences(refs),
 			});
 		}
</file context>

[cubic-dev-ai]

P1: When a review fails to load or the server reports the receipt as unavailable (not_found, content_changed, not_latest, reverted, review_unavailable), the document editor is left permanently frozen in read-only mode. The effect calls editor.setEditable(false) up front, but the error and 'not ready' branches call hideReview() and return before any cleanup is registered; since hideReview() nulls the target, the effect re-runs and exits through the if (!editor || !target) return; guard without ever restoring editor.setEditable(canEdit). The user is then stuck unable to edit that document until it is remounted. Consider restoring editability in those early-return branches (or always registering a cleanup that restores it) before calling hideReview().

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/use-document-edit-review-overlay.ts, line 46:

<comment>When a review fails to load or the server reports the receipt as unavailable (not_found, content_changed, not_latest, reverted, review_unavailable), the document editor is left permanently frozen in read-only mode. The effect calls `editor.setEditable(false)` up front, but the error and 'not ready' branches call `hideReview()` and `return` before any cleanup is registered; since `hideReview()` nulls the target, the effect re-runs and exits through the `if (!editor || !target) return;` guard without ever restoring `editor.setEditable(canEdit)`. The user is then stuck unable to edit that document until it is remounted. Consider restoring editability in those early-return branches (or always registering a cleanup that restores it) before calling hideReview().</comment>

<file context>
@@ -0,0 +1,84 @@
+		// Freeze on request rather than on arrival: the verdict takes a round trip,
+		// and a document that accepts typing in the meantime is one whose marks
+		// will not match it.
+		editor.setEditable(false);
+
+		if (reviewQuery.isError) {
</file context>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

This behavior change adds heading level 4 detection and renames the state union from DocumentFontSize to DocumentTextStyle, but no tests in the suite exercise getActiveBlock or the new textStyle values. A practical regression test (for example, mocking a TipTap editor and asserting the returned block for each heading level and the paragraph default) would prevent silent breakage of the toolbar state. Please add unit tests that cover the new heading4 branch and the semantic rename.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/document-editor/document-editor-state.ts, line 169:

<comment>This behavior change adds heading level 4 detection and renames the state union from `DocumentFontSize` to `DocumentTextStyle`, but no tests in the suite exercise `getActiveBlock` or the new `textStyle` values. A practical regression test (for example, mocking a TipTap editor and asserting the returned block for each heading level and the paragraph default) would prevent silent breakage of the toolbar state. Please add unit tests that cover the new `heading4` branch and the semantic rename.</comment>

<file context>
@@ -165,19 +166,13 @@ function getActiveBlock(editor: Editor): DocumentEditorUiState["block"] {
 
-	if (editor.isActive("heading", { level: 1 })) {
-		return { kind: "fontSize", size: "32" };
+	for (const level of [1, 2, 3, 4] as const) {
+		if (editor.isActive("heading", { level })) {
+			return { kind: "textStyle", style: `heading${level}` } as const;
</file context>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

The toolbar semantics changed from font-size-based actions to text-style-based actions (paragraph and heading levels 1–4), including a new Heading 4 action with its own active state, run command, and icon mapping. However, there is no visible test coverage exercising this new behavior. Consider adding unit tests that assert the isTextStyle active-state logic, the getTextStyleIcon mappings, and the TipTap commands invoked by each documentTextStyleActions entry—including the new heading level 4—to prevent regressions in the document editor toolbar.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/document-editor/document-editor-toolbar-actions.tsx, line 80:

<comment>The toolbar semantics changed from font-size-based actions to text-style-based actions (paragraph and heading levels 1–4), including a new Heading 4 action with its own `active` state, `run` command, and icon mapping. However, there is no visible test coverage exercising this new behavior. Consider adding unit tests that assert the `isTextStyle` active-state logic, the `getTextStyleIcon` mappings, and the TipTap commands invoked by each `documentTextStyleActions` entry—including the new heading level 4—to prevent regressions in the document editor toolbar.</comment>

<file context>
@@ -42,39 +43,47 @@ export interface DocumentToolbarAction {
 		run: (editor) => editor.chain().focus().setHeading({ level: 3 }).run(),
 	},
+	{
+		id: "text-style-heading4",
+		icon: <Heading4 />,
+		label: "Heading 4",
</file context>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

The modified persistence-based readiness condition introduces a real behavior change, but there's no visible test coverage for it. A session that has the cached 'server-synced' marker in IndexedDB but an empty tiptap XML fragment will no longer be marked ready early and will instead wait for the websocket sync event. This edge case is now silently altered without regression protection. Consider adding a test that mocks the IndexedDB marker and toggles an empty vs. non-empty Y.XmlFragment to assert whether markReady() fires without a websocket sync — this directly exercises the change and guards against regressions.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/use-document-collaboration-session.ts, line 207:

<comment>The modified persistence-based readiness condition introduces a real behavior change, but there's no visible test coverage for it. A session that has the cached 'server-synced' marker in IndexedDB but an empty tiptap XML fragment will no longer be marked ready early and will instead wait for the websocket sync event. This edge case is now silently altered without regression protection. Consider adding a test that mocks the IndexedDB marker and toggles an empty vs. non-empty `Y.XmlFragment` to assert whether `markReady()` fires without a websocket sync — this directly exercises the change and guards against regressions.</comment>

<file context>
@@ -203,7 +204,10 @@ function createActiveDocumentSession(input: {
 		.then(() => session.persistence.get(localDocumentReadyKey))
 		.then((wasServerSynced) => {
-			if (wasServerSynced === localDocumentReadyValue) {
+			if (
+				wasServerSynced === localDocumentReadyValue &&
+				session.ydoc.getXmlFragment(tiptapDocumentYjsField).length > 0
</file context>

[cubic-dev-ai]

P2: A turn-local ref can resolve to the wrong workspace item when the same short ref appears in more than one transcript turn, because the map overwrites earlier records without checking their locations. Preserve only unambiguous refs (matching the collision handling used by workspace-citations) before writing the citation.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/operations/document-citations.ts, line 28:

<comment>A turn-local ref can resolve to the wrong workspace item when the same short ref appears in more than one transcript turn, because the map overwrites earlier records without checking their locations. Preserve only unambiguous refs (matching the collision handling used by `workspace-citations`) before writing the citation.</comment>

<file context>
@@ -0,0 +1,30 @@
+
+	return applyDocumentCitationLocations(
+		input.html,
+		new Map(records.map((record) => [record.ref, record.location])),
+	);
+}
</file context>

[cubic-dev-ai]

P2: reconcileCurrentDocument now repopulates an existing Yjs XmlFragment without clearing it first, whereas the previous replaceCurrentDocument explicitly did fragment.delete(0, fragment.length) and wrapped the write in this.document.transact(...). If prosemirrorJSONToYXmlFragment appends (as the old code's explicit delete strongly suggests), every AI edit and every ref reconciliation would duplicate the document's top-level blocks, and dropping the transact wrapper also removes the atomicity the old code guaranteed when streaming updates to connected clients. Please verify the library clears the fragment and restore the explicit clear/transaction (as in the removed code) if it does not.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/document-session.ts, line 443:

<comment>`reconcileCurrentDocument` now repopulates an existing Yjs `XmlFragment` without clearing it first, whereas the previous `replaceCurrentDocument` explicitly did `fragment.delete(0, fragment.length)` and wrapped the write in `this.document.transact(...)`. If `prosemirrorJSONToYXmlFragment` appends (as the old code's explicit delete strongly suggests), every AI edit and every ref reconciliation would duplicate the document's top-level blocks, and dropping the `transact` wrapper also removes the atomicity the old code guaranteed when streaming updates to connected clients. Please verify the library clears the fragment and restore the explicit clear/transaction (as in the removed code) if it does not.</comment>

<file context>
@@ -230,32 +354,94 @@ export class DocumentSession extends YServer {
-			prosemirrorJSONToYXmlFragment(getTiptapDocumentSchema(), document, fragment);
-		}, this);
+	private reconcileCurrentDocument(document: TiptapDocumentJson) {
+		const fragment = this.document.getXmlFragment(tiptapDocumentYjsField);
+		prosemirrorJSONToYXmlFragment(getTiptapDocumentSchema(), document, fragment);
 	}
</file context>

[cubic-dev-ai]

P3: The round-trip test never verifies the round-trip: it only checks substrings and that re-parsing yields type: "doc", so silently dropped or reordered blocks still pass despite the test name and the PR's "remains lossless" claim. Consider asserting the re-parsed document deep-equals the original (or a normalized snapshot) to actually guard the round-trip.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/document-ai-html.test.ts, line 23:

<comment>The round-trip test never verifies the round-trip: it only checks substrings and that re-parsing yields `type: "doc"`, so silently dropped or reordered blocks still pass despite the test name and the PR's "remains lossless" claim. Consider asserting the re-parsed document deep-equals the original (or a normalized snapshot) to actually guard the round-trip.</comment>

<file context>
@@ -0,0 +1,71 @@
+		expect(html).toContain("<strong>bold</strong>");
+		expect(html).toContain('data-type="inline-math"');
+		expect(html).toContain('data-type="taskItem"');
+		expect(parseDocumentAiHtml(html)).toMatchObject({ type: "doc" });
+	});
+
</file context>

Suggested change

	expect(parseDocumentAiHtml(html)).toMatchObject({ type: "doc" });
	expect(parseDocumentAiHtml(html)).toEqual(document);

[cubic-dev-ai]

P3: Clicking a citation that points at an item that has since been deleted or renamed silently does nothing. The new click handler always calls event.preventDefault() and returns true as if the citation was actioned, but it discards the boolean reveal() returns (workspace-location-context.tsx returns Boolean(viewInstanceId), which is false when the target item no longer exists). Because the new styles.css gives citations cursor:pointer, a stale citation still looks clickable but produces no navigation and no feedback. Consider checking reveal()'s result (or hasItem) and, when it returns false, falling back to opening the plain document item or otherwise signalling that the source is unavailable, rather than swallowing the click.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/document-editor/DocumentEditorSurface.tsx, line 103:

<comment>Clicking a citation that points at an item that has since been deleted or renamed silently does nothing. The new click handler always calls event.preventDefault() and returns true as if the citation was actioned, but it discards the boolean reveal() returns (workspace-location-context.tsx returns Boolean(viewInstanceId), which is false when the target item no longer exists). Because the new styles.css gives citations cursor:pointer, a stale citation still looks clickable but produces no navigation and no feedback. Consider checking reveal()'s result (or hasItem) and, when it returns false, falling back to opening the plain document item or otherwise signalling that the source is unavailable, rather than swallowing the click.</comment>

<file context>
@@ -85,6 +89,26 @@ function DocumentEditorInstance({
+					}
+
+					const page = Number(citation?.getAttribute("data-page"));
+					event.preventDefault();
+					reveal(
+						Number.isInteger(page) && page > 0
</file context>

[cubic-dev-ai]

P3: The model-facing boundary strips app-only action from calls but does not strip the __thinkexUi/receipt metadata from result — it only stays clean because execute happens to feed it already-normalized (pre-stripped) output. If this exported helper is ever called with the raw runtime output (its signature accepts any output), the internal receipt ID would surface to the model. Applying stripAIThreadToolUiMetadata to result here (as normalizeAIThreadOrchestrationOutput already does) would make the boundary self-contained and consistent.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/ai/ai-thread-orchestration-contract.ts, line 208:

<comment>The model-facing boundary strips app-only `action` from calls but does not strip the `__thinkexUi`/receipt metadata from `result` — it only stays clean because `execute` happens to feed it already-normalized (pre-stripped) output. If this exported helper is ever called with the raw runtime output (its signature accepts any `output`), the internal receipt ID would surface to the model. Applying `stripAIThreadToolUiMetadata` to `result` here (as `normalizeAIThreadOrchestrationOutput` already does) would make the boundary self-contained and consistent.</comment>

<file context>
@@ -180,11 +194,25 @@ export function getAIThreadOrchestrationTelemetryOutput(output: unknown) {
+		return output;
+	}
+
+	return { ...parsed.data, calls: withoutCallActions(parsed.data.calls) };
+}
+
</file context>

[coderabbitai]

Actionable comments posted: 4

🧹 Nitpick comments (1)

src/styles.css (1)

668-684: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Provide a keyboard equivalent for the citation interaction.

cursor: pointer and :hover indicate an interactive citation, but these rules provide no :focus-visible state. If clicking a citation opens a source or details, render it as a keyboard-focusable link or button and handle keyboard activation. If citations are not interactive, remove the pointer cursor and hover rule.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/styles.css` around lines 668 - 684, Update the citation interaction
represented by the .workspace-document-prose citation styles to use a
keyboard-focusable link or button with keyboard activation, and add a clear
:focus-visible state matching the hover treatment. If citations are not actually
interactive, remove cursor: pointer and the citation:hover rule instead.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/features/workspaces/ai/ai-thread-runtime.ts`:
- Line 164: Update src/features/workspaces/ai/ai-thread-runtime.ts at line 164
so createAIThreadTurnToolConfig accepts and forwards resolveWorkspaceReferences
to createAIThreadToolCatalog; update src/features/workspaces/ai/ai-thread.ts at
line 175 so _createTurnToolConfig passes this._resolveWorkspaceReferences into
createAIThreadTurnToolConfig.

In `@src/features/workspaces/documents/document-ai-html.ts`:
- Around line 267-270: Update the element metadata handling in
parseDocumentAiHtml so non-"pdf-page" locations remove any existing data-page
attribute after setting data-item-id. Preserve setting data-page from
location.pageNumber for PDF-page citations, ensuring item-level citations do not
retain stale page values.

In `@src/features/workspaces/documents/document-edit-review-context.tsx`:
- Around line 44-69: Update the showReview callback to track a monotonically
increasing request identifier for each invocation, and capture the current
identifier before awaiting getDocumentEditReceiptReviewFn. After the request
resolves, ignore the result—including errors and status handling—unless its
identifier is still the latest, so an older request cannot call setActiveReview
or display stale feedback over a newer selection.

In `@src/styles.css`:
- Around line 668-684: Update the `.workspace-document-prose citation` and
corresponding `citation:hover` selectors to target the class or data attribute
emitted by the citation renderer instead of the unknown `citation` type
selector. Ensure both rules preserve their existing styles and satisfy Stylelint
without broad exceptions.

---

Nitpick comments:
In `@src/styles.css`:
- Around line 668-684: Update the citation interaction represented by the
.workspace-document-prose citation styles to use a keyboard-focusable link or
button with keyboard activation, and add a clear :focus-visible state matching
the hover treatment. If citations are not actually interactive, remove cursor:
pointer and the citation:hover rule instead.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: cc351221-ba83-4b84-a334-c73a05d84079

📥 Commits

Reviewing files that changed from the base of the PR and between cc160e9 and 0b0820a.

📒 Files selected for processing (20)

• src/features/workspaces/ai/ai-thread-runtime.ts
• src/features/workspaces/ai/ai-thread.ts
• src/features/workspaces/ai/workspace-tool-result-adapters.ts
• src/features/workspaces/ai/workspace-tools.ts
• src/features/workspaces/components/ai-chat/AiChatDocumentEditActions.tsx
• src/features/workspaces/components/document-editor/DocumentEditorSurface.tsx
• src/features/workspaces/documents/document-ai-edits.ts
• src/features/workspaces/documents/document-ai-html.ts
• src/features/workspaces/documents/document-edit-receipt.ts
• src/features/workspaces/documents/document-edit-review-context.tsx
• src/features/workspaces/documents/document-session.ts
• src/features/workspaces/documents/tiptap-schema.ts
• src/features/workspaces/documents/use-document-edit-receipt-undo.ts
• src/features/workspaces/documents/use-document-edit-review-overlay.ts
• src/features/workspaces/operations/create-items.ts
• src/features/workspaces/operations/document-citations.ts
• src/features/workspaces/operations/edit-item.ts
• src/features/workspaces/operations/workspace-access-context.ts
• src/features/workspaces/operations/workspace-tool-schemas.ts
• src/styles.css

🚧 Files skipped from review as they are similar to previous changes (6)

• src/features/workspaces/documents/use-document-edit-receipt-undo.ts
• src/features/workspaces/operations/edit-item.ts
• src/features/workspaces/ai/workspace-tool-result-adapters.ts
• src/features/workspaces/operations/workspace-tool-schemas.ts
• src/features/workspaces/documents/document-ai-edits.ts
• src/features/workspaces/documents/document-session.ts

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Pass resolveWorkspaceReferences through the orchestration path.

The direct tool catalog receives the resolver, but the turn tool catalog does not. As a result, codemode document operations cannot resolve wr_ citations and unresolved citations become plain text during document parsing.

• src/features/workspaces/ai/ai-thread-runtime.ts#L164-L164: Add resolveWorkspaceReferences to createAIThreadTurnToolConfig and forward it to createAIThreadToolCatalog.
• src/features/workspaces/ai/ai-thread.ts#L175-L175: Pass this._resolveWorkspaceReferences from _createTurnToolConfig to createAIThreadTurnToolConfig.

📍 Affects 2 files

• src/features/workspaces/ai/ai-thread-runtime.ts#L164-L164 (this comment)
• src/features/workspaces/ai/ai-thread.ts#L175-L175

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/features/workspaces/ai/ai-thread-runtime.ts` at line 164, Update
src/features/workspaces/ai/ai-thread-runtime.ts at line 164 so
createAIThreadTurnToolConfig accepts and forwards resolveWorkspaceReferences to
createAIThreadToolCatalog; update src/features/workspaces/ai/ai-thread.ts at
line 175 so _createTurnToolConfig passes this._resolveWorkspaceReferences into
createAIThreadTurnToolConfig.

[coderabbitai]

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Clear stale data-page metadata for non-PDF locations.

If the input citation already has data-page and location.kind is not "pdf-page", this code retains the old page value. parseDocumentAiHtml then stores a page number for an item-level citation.

Proposed fix

 		element.setAttribute("data-item-id", location.itemId);
 		if (location.kind === "pdf-page") {
 			element.setAttribute("data-page", String(location.pageNumber));
+		} else {
+			element.removeAttribute("data-page");
 		}

📝 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.

Suggested change

	element.setAttribute("data-item-id", location.itemId);
	if (location.kind === "pdf-page") {
	element.setAttribute("data-page", String(location.pageNumber));
	}
	element.setAttribute("data-item-id", location.itemId);
	if (location.kind === "pdf-page") {
	element.setAttribute("data-page", String(location.pageNumber));
	} else {
	element.removeAttribute("data-page");
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/features/workspaces/documents/document-ai-html.ts` around lines 267 -
270, Update the element metadata handling in parseDocumentAiHtml so
non-"pdf-page" locations remove any existing data-page attribute after setting
data-item-id. Preserve setting data-page from location.pageNumber for PDF-page
citations, ensuring item-level citations do not retain stale page values.

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Prevent an older review request from replacing a newer selection.

showReview calls can overlap. If request A starts, then request B starts, and A resolves last, Line 65 replaces B's active review with A's result. The open document can then have no overlay while another document receives the stale review state.

Track the latest request and ignore results from earlier requests.

Proposed fix

+const latestReviewRequest = useRef(0);
+
 const showReview = useCallback(
   async (input: { itemId: string; receiptIds: string[] }) => {
+    const requestId = ++latestReviewRequest.current;
     // reveal opens the document...
     if (!reveal({ itemId: input.itemId, kind: "item", version: 1 })) {
       toast.error("This document no longer exists.");
       return;
     }

     const review = await getDocumentEditReceiptReviewFn({
       data: { itemId: input.itemId, receiptIds: input.receiptIds, workspaceId },
     }).catch(() => null);

+    if (requestId !== latestReviewRequest.current) {
+      return;
+    }
+
     // Existing result handling.

📝 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.

Suggested change

	async (input: { itemId: string; receiptIds: string[] }) => {
	// reveal opens the document, or focuses the tab already holding it, and
	// only fails when the item is gone.
	if (!reveal({ itemId: input.itemId, kind: "item", version: 1 })) {
	toast.error("This document no longer exists.");
	return;
	}
	const review = await getDocumentEditReceiptReviewFn({
	data: { itemId: input.itemId, receiptIds: input.receiptIds, workspaceId },
	}).catch(() => null);
	if (!review) {
	toast.error("Could not load these changes.");
	return;
	}
	if (review.status !== "ready") {
	toast.error(unavailableReviewMessages[review.status]);
	return;
	}
	setActiveReview({
	beforeDocument: review.beforeDocument,
	itemId: input.itemId,
	receiptIds: input.receiptIds,
	});
	async (input: { itemId: string; receiptIds: string[] }) => {
	const requestId = ++latestReviewRequest.current;
	// reveal opens the document, or focuses the tab already holding it, and
	// only fails when the item is gone.
	if (!reveal({ itemId: input.itemId, kind: "item", version: 1 })) {
	toast.error("This document no longer exists.");
	return;
	}
	const review = await getDocumentEditReceiptReviewFn({
	data: { itemId: input.itemId, receiptIds: input.receiptIds, workspaceId },
	}).catch(() => null);
	if (requestId !== latestReviewRequest.current) {
	return;
	}
	if (!review) {
	toast.error("Could not load these changes.");
	return;
	}
	if (review.status !== "ready") {
	toast.error(unavailableReviewMessages[review.status]);
	return;
	}
	setActiveReview({
	beforeDocument: review.beforeDocument,
	itemId: input.itemId,
	receiptIds: input.receiptIds,
	});

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/features/workspaces/documents/document-edit-review-context.tsx` around
lines 44 - 69, Update the showReview callback to track a monotonically
increasing request identifier for each invocation, and capture the current
identifier before awaiting getDocumentEditReceiptReviewFn. After the request
resolves, ignore the result—including errors and status handling—unless its
identifier is still the latest, so an older request cannot call setActiveReview
or display stale feedback over a newer selection.

[coderabbitai]

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Fix the Stylelint error for citation.

citation is a non-standard type selector. Stylelint reports selector-type-no-unknown for both rules. Use a class or data-attribute selector emitted by the citation renderer, or add a narrow documented Stylelint exception.

🧰 Tools

🪛 Stylelint (17.14.1)

[error] 668-668: Unknown type selector "citation" (selector-type-no-unknown)

(selector-type-no-unknown)

---

[error] 681-681: Unknown type selector "citation" (selector-type-no-unknown)

(selector-type-no-unknown)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/styles.css` around lines 668 - 684, Update the `.workspace-document-prose
citation` and corresponding `citation:hover` selectors to target the class or
data attribute emitted by the citation renderer instead of the unknown
`citation` type selector. Ensure both rules preserve their existing styles and
satisfy Stylelint without broad exceptions.

Source: Linters/SAST tools

[cubic-dev-ai]

19 issues found across 58 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/features/workspaces/operations/create-items.ts">

<violation number="1" location="src/features/workspaces/operations/create-items.ts:121">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The `createWorkspaceItemsOperation` function now resolves citations via `resolveDocumentCitations` and parses `initialContent` as HTML through `parseDocumentAiHtml` (replacing the previous Markdown path). This is a meaningful behavior change: non-HTML content that was previously tolerated is now rejected as `invalid_initial_content`, and the `warnings` field has been removed entirely. However, there are no tests exercising this changed behavior in `create-items.ts`. Practical regression assertions — e.g., validating that HTML content succeeds, non-HTML returns `invalid_initial_content`, and citations are resolved during creation — are missing. Adding tests for the create-items path would ensure the new parsing and citation behavior is properly validated.</violation>

<violation number="2" location="src/features/workspaces/operations/create-items.ts:303">
P2: Created documents can contain fabricated citation targets: an AI-supplied `<citation data-item-id="...">` bypasses `resolveDocumentCitations` and is persisted as a clickable citation without a verified workspace reference. Normalize or reject `data-item-id` in model HTML and only retain IDs produced by resolving a valid `ref`.</violation>
</file>

<file name="src/features/workspaces/documents/use-document-edit-receipt-undo.ts">

<violation number="1" location="src/features/workspaces/documents/use-document-edit-receipt-undo.ts:24">
P2: Undoing one receipt can close a different document review opened while the request is pending. Make hiding the review conditional on the document/receipt that started the mutation, rather than clearing the provider-wide active review unconditionally.</violation>

<violation number="2" location="src/features/workspaces/documents/use-document-edit-receipt-undo.ts:26">
P2: A repeated or concurrent undo is shown as a fresh success even though no undo occurred. Return a distinct success/already-reverted status, or route the already-reverted result to the existing “These changes were already undone.” message.</violation>
</file>

<file name="src/features/workspaces/documents/document-edit-review-extension.ts">

<violation number="1" location="src/features/workspaces/documents/document-edit-review-extension.ts:74">
P3: While a review is open, every document transaction (including each remote collaborative edit) replays a full-document diff: createDocumentEditReviewDecorations re-parses the entire before-document JSON into a node and runs a whole-document ChangeSet comparison against the current content. For long documents with frequent collaborators this is repeated O(doc-size) work that could be done incrementally or memoized (cache the parsed before-node/schema outside the per-transaction path). Low risk for small docs, but worth guarding given the feature is explicitly scoped around larger documents elsewhere (review_unavailable).</violation>

<violation number="2" location="src/features/workspaces/documents/document-edit-review-extension.ts:167">
P2: Changes to atom blocks nested inside lists or blockquotes can appear with no review decoration, leaving users unable to see those AI edits. Traversing descendant nodes when finding changed atoms would preserve the intended block marker for nested content.</violation>
</file>

<file name="src/features/workspaces/documents/document-edit-review-context.tsx">

<violation number="1" location="src/features/workspaces/documents/document-edit-review-context.tsx:52">
P3: showReview races when the user opens reviews for two documents in quick succession. The RPC for the first document can resolve after the second one's, and setActiveReview simply overwrites, so the visible review and the on-screen document can end up out of sync (the overlay only applies when activeReview.itemId matches the mounted document). Consider guarding the async application—e.g., keep a ref of the most recently requested itemId and ignore any response that isn't for it, or abort/cancel the prior request before issuing a new one.</violation>
</file>

<file name="src/features/workspaces/documents/document-ai-edits.ts">

<violation number="1" location="src/features/workspaces/documents/document-ai-edits.ts:181">
P2: The receipt's line tally counts atom blocks (horizontal rule, math) by stringifying `node.toJSON()`, which includes the top-level `aiRef` attribute on every block. Because the `aiRef` fingerprint is part of the serialized line, an unchanged formula/hr block whose ref was regenerated by a `replace_all` edit will be counted as both a line removed and a line added, inflating the receipt's "Lines: +N −M" totals even though the block's content never changed. This is inconsistent with the text-block branch, which strips refs by using only `node.textContent`. Consider normalizing refs away before stringifying the atom, matching the `withoutTopLevelAiRefs`/`withTiptapNodeAiRef(node, null)` approach used elsewhere.</violation>
</file>

<file name="src/features/workspaces/content/workspace-content-reader.ts">

<violation number="1" location="src/features/workspaces/content/workspace-content-reader.ts:98">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The `readBudgetExhausted` flag changes the semantics of batch reading: after the first item exceeds the remaining byte budget, every later item is rejected without being read, even if it would fit. The PR does not document this behavior change, and the existing budget test (`bounds total content returned by a batch`) creates identical oversized items so it passes under both the old and new behavior. There is no test asserting the new hard-stop semantics, which means a small item after an oversized one is silently dropped. Consider either restoring the per-item budget check so smaller trailing items are still evaluated, or adding an explicit test that documents and asserts the new stop-after-first-rejection behavior.</violation>

<violation number="2" location="src/features/workspaces/content/workspace-content-reader.ts:107">
P3: The relations RPCs are now started without being awaited and only consumed in attachRelationPaths. In the normal path this is a fine parallelization. However, if a later read in the loop throws a non-WorkspacePageSelectionError (which is rethrown and aborts before attachRelationPaths runs), the in-flight listItemRelations promises for earlier ready items are left un-awaited and could surface as unhandled promise rejections if they reject. Consider collecting the promises and ensuring they are always awaited or attached a rejection handler so no kernel rejection is left dangling.</violation>
</file>

<file name="src/features/workspaces/documents/document-ai-html.ts">

<violation number="1" location="src/features/workspaces/documents/document-ai-html.ts:67">
P3: An unresolved citation (no data-item-id) that sits at the document root gets replaced by a plain-text node before validation, and then validateDocumentAiHtml rejects any non-whitespace top-level text node as "Plain text and Markdown are not accepted." So at the root level the documented design ("becomes its own label rather than failing the write") doesn't hold — the edit still fails, and with a misleading non-HTML message. If a resolver-less citation should degrade to its label, consider wrapping it in a paragraph (or skipping the top-level text check for known-good replacements) so the write doesn't fail here.</violation>

<violation number="2" location="src/features/workspaces/documents/document-ai-html.ts:268">
P2: When a citation already has `data-page` and the resolved location is not `pdf-page`, this branch leaves the old page attribute in place. That stale metadata can make an item-level citation look page-specific on later parse/read cycles. Clearing `data-page` in the non-PDF case would prevent that carry-over.</violation>
</file>

<file name="src/features/workspaces/documents/document-session.ts">

<violation number="1" location="src/features/workspaces/documents/document-session.ts:442">
P1: The new `reconcileCurrentDocument` replaces the old `replaceCurrentDocument`, but it drops the explicit `fragment.delete(0, fragment.length)` (and the `transact` wrapper) that the previous implementation performed before calling `prosemirrorJSONToYXmlFragment`. Since every caller of this helper (applyEdits, undoDocumentEditReceipt, getReferencedDocumentSnapshot) writes into a fragment that already holds content, correct replacement depends on `prosemirrorJSONToYXmlFragment` clearing the target fragment itself — which is not guaranteed by the library contract. If it merely appends/inserts, each AI edit or undo would silently duplicate the document content. Please confirm the library behavior and, if it does not clear, restore an explicit piecewise clear (ideally inside a transaction, as before) before merging.</violation>
</file>

<file name="src/features/workspaces/documents/document-edit-review-functions.ts">

<violation number="1" location="src/features/workspaces/documents/document-edit-review-functions.ts:23">
P2: This review RPC is a GET but can carry a large payload. The schema allows up to 40 receipt ids, each up to 512 characters, and TanStack Start encodes the validator data into the URL for GET requests, so a receipt-heavy turn can push the query string toward a size that trips intermediary URL-length limits and fail to load the review silently. Using method: "POST" (undo already is) avoids the URL-size constraint, or the caps can be tightened.</violation>
</file>

<file name="src/features/workspaces/documents/use-document-collaboration-session.ts">

<violation number="1" location="src/features/workspaces/documents/use-document-collaboration-session.ts:209">
P1: Custom agent: **Flag AI Slop and Fabricated Changes**

This PR changes the document-session readiness logic by adding an extra `session.ydoc.getXmlFragment(tiptapDocumentYjsField).length > 0` check to the IndexedDB cache path. Previously, a cached `server-synced` flag alone would mark the session ready; now an empty Yjs fragment prevents that. If a user is offline with an empty or newly-created previously-synced document, the session will never become ready because the provider `sync` handler won't fire and the IndexedDB cache path is now gated. This is a real behavior change with a clear offline regression, yet no tests cover this file or the new condition. Please add a regression test asserting that a non-empty fragment with a cached sync flag marks ready, while an empty fragment with the same flag defers readiness.</violation>
</file>

<file name="src/features/workspaces/components/document-editor/document-editor-toolbar-actions.tsx">

<violation number="1" location="src/features/workspaces/components/document-editor/document-editor-toolbar-actions.tsx:79">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The new `heading4` toolbar action and the broader refactor from `fontSize` to `textStyle` block kinds are user-visible behavior changes, yet no tests cover these additions. The `document-editor` component directory has no test files at all, and no regression-style assertions verify that `documentTextStyleActions` includes `heading4`, that `getTextStyleIcon` returns the correct icon, or that `isTextStyle` matches the new `textStyle` kind. Practical unit tests for these exports would be straightforward and should be added to guard against future regressions.</violation>
</file>

<file name="src/features/workspaces/ai/ai-thread-tool-ui-metadata.ts">

<violation number="1" location="src/features/workspaces/ai/ai-thread-tool-ui-metadata.ts:7">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The new `ai-thread-tool-ui-metadata.ts` module introduces runtime behavior for attaching, reading, and stripping AI tool output metadata, but has no test coverage. Regression-style assertions are very practical for these pure utilities (attach→get round-trip, recursive strip at various nesting depths, pass-through for non-objects). Please add unit tests for `attachDocumentEditReceiptMetadata`, `getDocumentEditReceiptMetadata`, and `stripAIThreadToolUiMetadata`.</violation>
</file>

<file name="src/features/workspaces/ai/ai-thread-orchestration.ts">

<violation number="1" location="src/features/workspaces/ai/ai-thread-orchestration.ts:80">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

This PR adds a new `toModelOutput` property to the orchestration tool that wraps `getAIThreadOrchestrationModelOutput`, but no test verifies that the returned tool actually includes this property or that it returns the expected `{ type: "json", value: ... }` shape. While the underlying helper is tested separately, the integration point is not. A regression assertion here is practical and would prevent silent breakage of the model-output projection.</violation>

<violation number="2" location="src/features/workspaces/ai/ai-thread-orchestration.ts:133">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The connector now conditionally attaches document-edit receipt metadata for `workspace_edit_item`, but this execution path is not exercised by any visible test. The `ai-thread-orchestration.worker.test.ts` mocks `createExecuteRuntime` entirely and only tests a generic "nested" tool through `connector.tools()`, not `workspace_edit_item`. `attachDocumentEditReceiptMetadata` has zero references in test files. A practical regression test would exercise `workspace_edit_item` through the connector and assert receipt metadata is present, while verifying other tools do not receive it.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P1: The new reconcileCurrentDocument replaces the old replaceCurrentDocument, but it drops the explicit fragment.delete(0, fragment.length) (and the transact wrapper) that the previous implementation performed before calling prosemirrorJSONToYXmlFragment. Since every caller of this helper (applyEdits, undoDocumentEditReceipt, getReferencedDocumentSnapshot) writes into a fragment that already holds content, correct replacement depends on prosemirrorJSONToYXmlFragment clearing the target fragment itself — which is not guaranteed by the library contract. If it merely appends/inserts, each AI edit or undo would silently duplicate the document content. Please confirm the library behavior and, if it does not clear, restore an explicit piecewise clear (ideally inside a transaction, as before) before merging.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/document-session.ts, line 442:

<comment>The new `reconcileCurrentDocument` replaces the old `replaceCurrentDocument`, but it drops the explicit `fragment.delete(0, fragment.length)` (and the `transact` wrapper) that the previous implementation performed before calling `prosemirrorJSONToYXmlFragment`. Since every caller of this helper (applyEdits, undoDocumentEditReceipt, getReferencedDocumentSnapshot) writes into a fragment that already holds content, correct replacement depends on `prosemirrorJSONToYXmlFragment` clearing the target fragment itself — which is not guaranteed by the library contract. If it merely appends/inserts, each AI edit or undo would silently duplicate the document content. Please confirm the library behavior and, if it does not clear, restore an explicit piecewise clear (ideally inside a transaction, as before) before merging.</comment>

<file context>
@@ -230,32 +354,94 @@ export class DocumentSession extends YServer {
-			fragment.delete(0, fragment.length);
-			prosemirrorJSONToYXmlFragment(getTiptapDocumentSchema(), document, fragment);
-		}, this);
+	private reconcileCurrentDocument(document: TiptapDocumentJson) {
+		const fragment = this.document.getXmlFragment(tiptapDocumentYjsField);
+		prosemirrorJSONToYXmlFragment(getTiptapDocumentSchema(), document, fragment);
</file context>

[cubic-dev-ai]

P1: Custom agent: Flag AI Slop and Fabricated Changes

This PR changes the document-session readiness logic by adding an extra session.ydoc.getXmlFragment(tiptapDocumentYjsField).length > 0 check to the IndexedDB cache path. Previously, a cached server-synced flag alone would mark the session ready; now an empty Yjs fragment prevents that. If a user is offline with an empty or newly-created previously-synced document, the session will never become ready because the provider sync handler won't fire and the IndexedDB cache path is now gated. This is a real behavior change with a clear offline regression, yet no tests cover this file or the new condition. Please add a regression test asserting that a non-empty fragment with a cached sync flag marks ready, while an empty fragment with the same flag defers readiness.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/use-document-collaboration-session.ts, line 209:

<comment>This PR changes the document-session readiness logic by adding an extra `session.ydoc.getXmlFragment(tiptapDocumentYjsField).length > 0` check to the IndexedDB cache path. Previously, a cached `server-synced` flag alone would mark the session ready; now an empty Yjs fragment prevents that. If a user is offline with an empty or newly-created previously-synced document, the session will never become ready because the provider `sync` handler won't fire and the IndexedDB cache path is now gated. This is a real behavior change with a clear offline regression, yet no tests cover this file or the new condition. Please add a regression test asserting that a non-empty fragment with a cached sync flag marks ready, while an empty fragment with the same flag defers readiness.</comment>

<file context>
@@ -203,7 +204,10 @@ function createActiveDocumentSession(input: {
-			if (wasServerSynced === localDocumentReadyValue) {
+			if (
+				wasServerSynced === localDocumentReadyValue &&
+				session.ydoc.getXmlFragment(tiptapDocumentYjsField).length > 0
+			) {
 				markReady();
</file context>

[cubic-dev-ai]

P2: Created documents can contain fabricated citation targets: an AI-supplied <citation data-item-id="..."> bypasses resolveDocumentCitations and is persisted as a clickable citation without a verified workspace reference. Normalize or reject data-item-id in model HTML and only retain IDs produced by resolving a valid ref.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/operations/create-items.ts, line 303:

<comment>Created documents can contain fabricated citation targets: an AI-supplied `<citation data-item-id="...">` bypasses `resolveDocumentCitations` and is persisted as a clickable citation without a verified workspace reference. Normalize or reject `data-item-id` in model HTML and only retain IDs produced by resolving a valid `ref`.</comment>

<file context>
@@ -293,12 +299,9 @@ function getCreateWorkspaceItemInitialContent(input: CreateWorkspaceItemOperatio
-
 		return {
-			content: stringifyTiptapDocumentJson(projection.document),
+			content: stringifyTiptapDocumentJson(parseDocumentAiHtml(input.initialContent)),
 			status: "ready",
-			...(projection.warnings.length > 0 ? { warnings: projection.warnings } : {}),
</file context>

[cubic-dev-ai]

P2: A repeated or concurrent undo is shown as a fresh success even though no undo occurred. Return a distinct success/already-reverted status, or route the already-reverted result to the existing “These changes were already undone.” message.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/use-document-edit-receipt-undo.ts, line 26:

<comment>A repeated or concurrent undo is shown as a fresh success even though no undo occurred. Return a distinct success/already-reverted status, or route the already-reverted result to the existing “These changes were already undone.” message.</comment>

<file context>
@@ -0,0 +1,44 @@
+			// after that would be showing a diff we have just been told is wrong.
+			hideReview();
+
+			if (result.status === "reverted") {
+				toast.success("Changes undone.");
+			} else {
</file context>

[cubic-dev-ai]

P2: Undoing one receipt can close a different document review opened while the request is pending. Make hiding the review conditional on the document/receipt that started the mutation, rather than clearing the provider-wide active review unconditionally.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/use-document-edit-receipt-undo.ts, line 24:

<comment>Undoing one receipt can close a different document review opened while the request is pending. Make hiding the review conditional on the document/receipt that started the mutation, rather than clearing the provider-wide active review unconditionally.</comment>

<file context>
@@ -0,0 +1,44 @@
+			// Every outcome ends the review: it either just undid the changes, or
+			// told us they no longer describe the document. Leaving the marks up
+			// after that would be showing a diff we have just been told is wrong.
+			hideReview();
+
+			if (result.status === "reverted") {
</file context>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

The readBudgetExhausted flag changes the semantics of batch reading: after the first item exceeds the remaining byte budget, every later item is rejected without being read, even if it would fit. The PR does not document this behavior change, and the existing budget test (bounds total content returned by a batch) creates identical oversized items so it passes under both the old and new behavior. There is no test asserting the new hard-stop semantics, which means a small item after an oversized one is silently dropped. Consider either restoring the per-item budget check so smaller trailing items are still evaluated, or adding an explicit test that documents and asserts the new stop-after-first-rejection behavior.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/content/workspace-content-reader.ts, line 98:

<comment>The `readBudgetExhausted` flag changes the semantics of batch reading: after the first item exceeds the remaining byte budget, every later item is rejected without being read, even if it would fit. The PR does not document this behavior change, and the existing budget test (`bounds total content returned by a batch`) creates identical oversized items so it passes under both the old and new behavior. There is no test asserting the new hard-stop semantics, which means a small item after an oversized one is silently dropped. Consider either restoring the per-item budget check so smaller trailing items are still evaluated, or adding an explicit test that documents and asserts the new stop-after-first-rejection behavior.</comment>

<file context>
@@ -86,20 +95,16 @@ export async function readWorkspaceContent(input: {
-					status: "failed",
-					...(resolution.item.type === "file" ? { type: "file" as const } : {}),
-				});
+				readBudgetExhausted = true;
+				results.push(readBudgetFailure);
 				continue;
</file context>

[cubic-dev-ai]

P3: showReview races when the user opens reviews for two documents in quick succession. The RPC for the first document can resolve after the second one's, and setActiveReview simply overwrites, so the visible review and the on-screen document can end up out of sync (the overlay only applies when activeReview.itemId matches the mounted document). Consider guarding the async application—e.g., keep a ref of the most recently requested itemId and ignore any response that isn't for it, or abort/cancel the prior request before issuing a new one.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/document-edit-review-context.tsx, line 52:

<comment>showReview races when the user opens reviews for two documents in quick succession. The RPC for the first document can resolve after the second one's, and setActiveReview simply overwrites, so the visible review and the on-screen document can end up out of sync (the overlay only applies when activeReview.itemId matches the mounted document). Consider guarding the async application—e.g., keep a ref of the most recently requested itemId and ignore any response that isn't for it, or abort/cancel the prior request before issuing a new one.</comment>

<file context>
@@ -0,0 +1,96 @@
+				return;
+			}
+
+			const review = await getDocumentEditReceiptReviewFn({
+				data: { itemId: input.itemId, receiptIds: input.receiptIds, workspaceId },
+			}).catch(() => null);
</file context>

[cubic-dev-ai]

P3: The relations RPCs are now started without being awaited and only consumed in attachRelationPaths. In the normal path this is a fine parallelization. However, if a later read in the loop throws a non-WorkspacePageSelectionError (which is rethrown and aborts before attachRelationPaths runs), the in-flight listItemRelations promises for earlier ready items are left un-awaited and could surface as unhandled promise rejections if they reject. Consider collecting the promises and ensuring they are always awaited or attached a rejection handler so no kernel rejection is left dangling.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/content/workspace-content-reader.ts, line 107:

<comment>The relations RPCs are now started without being awaited and only consumed in attachRelationPaths. In the normal path this is a fine parallelization. However, if a later read in the loop throws a non-WorkspacePageSelectionError (which is rethrown and aborts before attachRelationPaths runs), the in-flight listItemRelations promises for earlier ready items are left un-awaited and could surface as unhandled promise rejections if they reject. Consider collecting the promises and ensuring they are always awaited or attached a rejection handler so no kernel rejection is left dangling.</comment>

<file context>
@@ -86,20 +95,16 @@ export async function readWorkspaceContent(input: {
 				item: resolution.item,
 				read,
-				relations: await input.kernel.listItemRelations({ itemId: resolution.item.id }),
+				relations: input.kernel.listItemRelations({ itemId: resolution.item.id }),
 			};
 			readyResults.push(pending);
</file context>

[cubic-dev-ai]

P3: An unresolved citation (no data-item-id) that sits at the document root gets replaced by a plain-text node before validation, and then validateDocumentAiHtml rejects any non-whitespace top-level text node as "Plain text and Markdown are not accepted." So at the root level the documented design ("becomes its own label rather than failing the write") doesn't hold — the edit still fails, and with a misleading non-HTML message. If a resolver-less citation should degrade to its label, consider wrapping it in a paragraph (or skipping the top-level text check for known-good replacements) so the write doesn't fail here.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/document-ai-html.ts, line 67:

<comment>An unresolved citation (no data-item-id) that sits at the document root gets replaced by a plain-text node before validation, and then validateDocumentAiHtml rejects any non-whitespace top-level text node as "Plain text and Markdown are not accepted." So at the root level the documented design ("becomes its own label rather than failing the write") doesn't hold — the edit still fails, and with a misleading non-HTML message. If a resolver-less citation should degrade to its label, consider wrapping it in a paragraph (or skipping the top-level text check for known-good replacements) so the write doesn't fail here.</comment>

<file context>
@@ -0,0 +1,278 @@
+
+	// A citation the operation could not resolve to a real item cannot navigate
+	// anywhere, so it becomes its own label rather than failing the write.
+	for (const element of htmlDocument.body.querySelectorAll("citation:not([data-item-id])")) {
+		element.replaceWith(htmlDocument.createTextNode(element.textContent ?? ""));
+	}
</file context>

[cubic-dev-ai]

P3: While a review is open, every document transaction (including each remote collaborative edit) replays a full-document diff: createDocumentEditReviewDecorations re-parses the entire before-document JSON into a node and runs a whole-document ChangeSet comparison against the current content. For long documents with frequent collaborators this is repeated O(doc-size) work that could be done incrementally or memoized (cache the parsed before-node/schema outside the per-transaction path). Low risk for small docs, but worth guarding given the feature is explicitly scoped around larger documents elsewhere (review_unavailable).

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/document-edit-review-extension.ts, line 74:

<comment>While a review is open, every document transaction (including each remote collaborative edit) replays a full-document diff: createDocumentEditReviewDecorations re-parses the entire before-document JSON into a node and runs a whole-document ChangeSet comparison against the current content. For long documents with frequent collaborators this is repeated O(doc-size) work that could be done incrementally or memoized (cache the parsed before-node/schema outside the per-transaction path). Low risk for small docs, but worth guarding given the feature is explicitly scoped around larger documents elsewhere (review_unavailable).</comment>

<file context>
@@ -0,0 +1,204 @@
+							return review;
+						}
+
+						return {
+							beforeDocument,
+							decorations: createDocumentEditReviewDecorations(beforeDocument, newState.doc),
</file context>

[github-actions]

React Doctor · react-doctor/js-combine-iterations (warning)

This loops over your list twice because .filter().map() makes two passes, so do it in one pass with .reduce() or a for...of loop

Fix → Combine .map().filter() style chains into one pass with .reduce() or a for...of loop, so you only loop over the list once

Docs

[github-actions]

React Doctor · react-doctor/js-set-map-lookups (warning)

This scales poorly because array.includes() inside a loop scans the whole list every time. Use a Set for constant-time lookups.

Fix → Use a Set or Map when you check for the same items over and over. Array.includes/find scans the whole list each time

Docs

[github-actions]

React Doctor · react-doctor/query-mutation-missing-invalidation (warning)

useMutation with no cache update here can leave your users looking at stale data after it runs.

Fix → Add onSuccess: () => queryClient.invalidateQueries({ queryKey: ['...'] }) so cached data stays in sync after the mutation

Docs

[cubic-dev-ai]

11 issues found and verified against the latest diff

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/features/workspaces/documents/document-edit-review-functions.ts">

<violation number="1" location="src/features/workspaces/documents/document-edit-review-functions.ts:40">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The PR introduces new server functions `getDocumentEditReceiptReviewFn` and `undoDocumentEditReceiptFn` that implement the core review/undo behavior for AI document edits, but no tests exercise these handlers. Adding regression-style tests for the validation schema, authorization branching (`read` vs `mutate`), and the delegation to the document session would be practical and would help prevent regressions in this new workflow.</violation>
</file>

<file name="src/features/workspaces/operations/create-items.ts">

<violation number="1" location="src/features/workspaces/operations/create-items.ts:125">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The behavioral change in how initial document content is processed — now flowing through `resolveDocumentCitations` and `parseDocumentAiHtml` instead of the old Markdown projection — is not exercised by any tests for this operation. I found no test files covering `create-items.ts`, `createWorkspaceItemsOperation`, `resolveDocumentCitations`, or `initialContent`. While the underlying parser has unit tests, the integration path in this file is entirely untested, and a regression-style assertion (e.g., creating an item with HTML content and asserting successful parsing and no warnings) is practical. Consider adding tests that exercise the new HTML/citation pipeline through `createWorkspaceItemsOperation` and assert the removal of `warnings` from the result.</violation>
</file>

<file name="src/features/workspaces/documents/document-edit-review-extension.ts">

<violation number="1" location="src/features/workspaces/documents/document-edit-review-extension.ts:76">
P3: While a review is active the document is read-only, but it is still collaborative — any remote teammate typing arrives as a `docChanged` transaction, and the plugin's `apply` recomputes a full-document `ChangeSet` diff (plus a per-change `document.forEach` in `addChangedAtomDecorations`) on every such transaction. For a large or actively-edited document this is a meaningful recompute repeated for the whole duration of the review. Consider caching/recomputing the decorations only when the review is shown (or at most debounced/throttled), since interacting with the review itself doesn't change the doc while it is read-only.</violation>

<violation number="2" location="src/features/workspaces/documents/document-edit-review-extension.ts:111">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The new review-mark derivation logic (ChangeSet diffing, custom TokenEncoder, `simplifyChanges`, and decoration creation for insertions, deletions, and atom blocks) is not accompanied by tests in this diff. Behavior-change/feature PRs should exercise changed behavior with assertions rather than leaving complex logic unverified. Consider adding unit tests that exercise `createDocumentEditReviewDecorations` with concrete before/after documents and assert the resulting `DecorationSet` for insertions, deletions, attribute-only changes, and atom-block changes.</violation>
</file>

<file name="src/features/workspaces/ai/ai-thread.ts">

<violation number="1" location="src/features/workspaces/ai/ai-thread.ts:521">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The new `_resolveWorkspaceReferences` method combines transcript references with in-flight `activeWorkspaceReferences` to let tools resolve refs from the current turn before they hit the transcript. This is a non-trivial behavior change in a behavior-change PR, yet no visible tests exercise this path. A regression-style test (e.g., simulating a document read followed by a ref citation within the same turn) would be practical and should be added to verify the merging logic works correctly.</violation>
</file>

<file name="src/features/workspaces/documents/use-document-collaboration-session.ts">

<violation number="1" location="src/features/workspaces/documents/use-document-collaboration-session.ts:209">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The added empty-fragment guard changes when a collaboration session is marked ready, introducing a new edge case where a previously-synced but empty document never reaches ready until a server round-trip. A regression-style test is practical here (e.g. asserting that `markReady` is withheld when the IndexedDB marker is present but the Yjs XML fragment is empty), yet no tests cover this file or the readiness logic. Consider adding a test for this guard so future changes to the restore path have coverage.</violation>
</file>

<file name="src/features/workspaces/documents/document-edit-review-context.tsx">

<violation number="1" location="src/features/workspaces/documents/document-edit-review-context.tsx:43">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The `showReview` callback and related review-state logic in this new context file are not covered by any unit test. Because this is a behavior-change PR introducing a review/undo workflow, Rule 1 expects new behavior to be exercised by tests when practical. Consider adding tests that cover the async `showReview` flow (successful load, each unavailable status mapping to the correct toast message, and state transitions) as well as `hideReview` and the missing-provider error path.</violation>

<violation number="2" location="src/features/workspaces/documents/document-edit-review-context.tsx:45">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The comment claims `reveal` "only fails when the item is gone," but `reveal` simply returns `Boolean(navigate(location))` where `navigate` is an external dependency. It could fail for layout or navigation reasons unrelated to item existence, and the context already exposes a separate `hasItem` for that purpose. The resulting toast message therefore may misattribute the failure. Consider removing the unfounded "only fails" claim so the comment describes behavior without asserting a contract that isn't guaranteed.</violation>

<violation number="3" location="src/features/workspaces/documents/document-edit-review-context.tsx:65">
P2: A slower earlier review request can replace the review selected by a later click, or reopen a review after the user presses Done, because every response unconditionally calls `setActiveReview`. Tracking a request generation and invalidating it from `hideReview` would ensure only the latest request can commit state.</violation>
</file>

<file name="src/features/workspaces/ai/ai-thread-tool-ui-metadata.ts">

<violation number="1" location="src/features/workspaces/ai/ai-thread-tool-ui-metadata.ts:15">
P2: Receipt IDs intended for app-only review controls are exposed to model-generated Code Mode code and can be echoed under another property, bypassing the metadata stripper. Keeping the receipt in a server-side/call side channel, or otherwise removing it before the sandbox sees the result, would preserve the app-only boundary.</violation>
</file>

<file name="src/features/workspaces/documents/use-document-edit-receipt-undo.ts">

<violation number="1" location="src/features/workspaces/documents/use-document-edit-receipt-undo.ts:24">
P2: A delayed undo response can close the wrong document's review. Make the hide conditional on the active review still matching this undo target (or use a review token) so switching receipts while the request is pending does not discard the new review.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

The PR introduces new server functions getDocumentEditReceiptReviewFn and undoDocumentEditReceiptFn that implement the core review/undo behavior for AI document edits, but no tests exercise these handlers. Adding regression-style tests for the validation schema, authorization branching (read vs mutate), and the delegation to the document session would be practical and would help prevent regressions in this new workflow.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/document-edit-review-functions.ts, line 40:

<comment>The PR introduces new server functions `getDocumentEditReceiptReviewFn` and `undoDocumentEditReceiptFn` that implement the core review/undo behavior for AI document edits, but no tests exercise these handlers. Adding regression-style tests for the validation schema, authorization branching (`read` vs `mutate`), and the delegation to the document session would be practical and would help prevent regressions in this new workflow.</comment>

<file context>
@@ -0,0 +1,74 @@
+			: result;
+	});
+
+export const undoDocumentEditReceiptFn = createServerFn({ method: "POST" })
+	.validator(documentEditReceiptInputSchema)
+	.handler(async ({ data }): Promise<DocumentEditReceiptUndoResult> => {
</file context>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

The behavioral change in how initial document content is processed — now flowing through resolveDocumentCitations and parseDocumentAiHtml instead of the old Markdown projection — is not exercised by any tests for this operation. I found no test files covering create-items.ts, createWorkspaceItemsOperation, resolveDocumentCitations, or initialContent. While the underlying parser has unit tests, the integration path in this file is entirely untested, and a regression-style assertion (e.g., creating an item with HTML content and asserting successful parsing and no warnings) is practical. Consider adding tests that exercise the new HTML/citation pipeline through createWorkspaceItemsOperation and assert the removal of warnings from the result.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/operations/create-items.ts, line 125:

<comment>The behavioral change in how initial document content is processed — now flowing through `resolveDocumentCitations` and `parseDocumentAiHtml` instead of the old Markdown projection — is not exercised by any tests for this operation. I found no test files covering `create-items.ts`, `createWorkspaceItemsOperation`, `resolveDocumentCitations`, or `initialContent`. While the underlying parser has unit tests, the integration path in this file is entirely untested, and a regression-style assertion (e.g., creating an item with HTML content and asserting successful parsing and no warnings) is practical. Consider adding tests that exercise the new HTML/citation pipeline through `createWorkspaceItemsOperation` and assert the removal of `warnings` from the result.</comment>

<file context>
@@ -118,7 +118,18 @@ export async function createWorkspaceItemsOperation(
+			itemInput.type === "document" && itemInput.initialContent !== undefined
+				? {
+						...itemInput,
+						initialContent: await resolveDocumentCitations({
+							context: accessContext,
+							html: itemInput.initialContent,
</file context>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

The new review-mark derivation logic (ChangeSet diffing, custom TokenEncoder, simplifyChanges, and decoration creation for insertions, deletions, and atom blocks) is not accompanied by tests in this diff. Behavior-change/feature PRs should exercise changed behavior with assertions rather than leaving complex logic unverified. Consider adding unit tests that exercise createDocumentEditReviewDecorations with concrete before/after documents and assert the resulting DecorationSet for insertions, deletions, attribute-only changes, and atom-block changes.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/document-edit-review-extension.ts, line 111:

<comment>The new review-mark derivation logic (ChangeSet diffing, custom TokenEncoder, `simplifyChanges`, and decoration creation for insertions, deletions, and atom blocks) is not accompanied by tests in this diff. Behavior-change/feature PRs should exercise changed behavior with assertions rather than leaving complex logic unverified. Consider adding unit tests that exercise `createDocumentEditReviewDecorations` with concrete before/after documents and assert the resulting `DecorationSet` for insertions, deletions, attribute-only changes, and atom-block changes.</comment>

<file context>
@@ -0,0 +1,204 @@
+	);
+}
+
+function createDocumentEditReviewDecorations(
+	beforeDocument: TiptapDocumentJson,
+	afterDocument: ProseMirrorNode,
</file context>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

The new _resolveWorkspaceReferences method combines transcript references with in-flight activeWorkspaceReferences to let tools resolve refs from the current turn before they hit the transcript. This is a non-trivial behavior change in a behavior-change PR, yet no visible tests exercise this path. A regression-style test (e.g., simulating a document read followed by a ref citation within the same turn) would be practical and should be added to verify the merging logic works correctly.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/ai/ai-thread.ts, line 521:

<comment>The new `_resolveWorkspaceReferences` method combines transcript references with in-flight `activeWorkspaceReferences` to let tools resolve refs from the current turn before they hit the transcript. This is a non-trivial behavior change in a behavior-change PR, yet no visible tests exercise this path. A regression-style test (e.g., simulating a document read followed by a ref citation within the same turn) would be practical and should be added to verify the merging logic works correctly.</comment>

<file context>
@@ -517,6 +518,21 @@ export function createAIThreadClass(getUserAIStore: () => typeof UserAIStore) {
 			this.activeWorkspaceReferences.push(...records);
 		}
 
+		/**
+		 * Looks up the refs a read handed the assistant, so a tool can turn one
+		 * into the location it stands for. Reads from this turn are not in the
</file context>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

The added empty-fragment guard changes when a collaboration session is marked ready, introducing a new edge case where a previously-synced but empty document never reaches ready until a server round-trip. A regression-style test is practical here (e.g. asserting that markReady is withheld when the IndexedDB marker is present but the Yjs XML fragment is empty), yet no tests cover this file or the readiness logic. Consider adding a test for this guard so future changes to the restore path have coverage.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/use-document-collaboration-session.ts, line 209:

<comment>The added empty-fragment guard changes when a collaboration session is marked ready, introducing a new edge case where a previously-synced but empty document never reaches ready until a server round-trip. A regression-style test is practical here (e.g. asserting that `markReady` is withheld when the IndexedDB marker is present but the Yjs XML fragment is empty), yet no tests cover this file or the readiness logic. Consider adding a test for this guard so future changes to the restore path have coverage.</comment>

<file context>
@@ -203,7 +204,10 @@ function createActiveDocumentSession(input: {
-			if (wasServerSynced === localDocumentReadyValue) {
+			if (
+				wasServerSynced === localDocumentReadyValue &&
+				session.ydoc.getXmlFragment(tiptapDocumentYjsField).length > 0
+			) {
 				markReady();
</file context>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

The showReview callback and related review-state logic in this new context file are not covered by any unit test. Because this is a behavior-change PR introducing a review/undo workflow, Rule 1 expects new behavior to be exercised by tests when practical. Consider adding tests that cover the async showReview flow (successful load, each unavailable status mapping to the correct toast message, and state transitions) as well as hideReview and the missing-provider error path.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/document-edit-review-context.tsx, line 43:

<comment>The `showReview` callback and related review-state logic in this new context file are not covered by any unit test. Because this is a behavior-change PR introducing a review/undo workflow, Rule 1 expects new behavior to be exercised by tests when practical. Consider adding tests that cover the async `showReview` flow (successful load, each unavailable status mapping to the correct toast message, and state transitions) as well as `hideReview` and the missing-provider error path.</comment>

<file context>
@@ -0,0 +1,96 @@
+	const { reveal } = useWorkspaceLocationActions();
+	const [activeReview, setActiveReview] = useState<ActiveDocumentEditReview | null>(null);
+	const hideReview = useCallback(() => setActiveReview(null), []);
+	const showReview = useCallback(
+		async (input: { itemId: string; receiptIds: string[] }) => {
+			// reveal opens the document, or focuses the tab already holding it, and
</file context>

[cubic-dev-ai]

P2: Receipt IDs intended for app-only review controls are exposed to model-generated Code Mode code and can be echoed under another property, bypassing the metadata stripper. Keeping the receipt in a server-side/call side channel, or otherwise removing it before the sandbox sees the result, would preserve the app-only boundary.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/ai/ai-thread-tool-ui-metadata.ts, line 15:

<comment>Receipt IDs intended for app-only review controls are exposed to model-generated Code Mode code and can be echoed under another property, bypassing the metadata stripper. Keeping the receipt in a server-side/call side channel, or otherwise removing it before the sandbox sees the result, would preserve the app-only boundary.</comment>

<file context>
@@ -0,0 +1,52 @@
+	return {
+		...output,
+		[aiThreadToolUiMetadataKey]: {
+			documentEditReceiptId: receiptId,
+		} satisfies AIThreadToolUiMetadata,
+	};
</file context>

[cubic-dev-ai]

P2: A slower earlier review request can replace the review selected by a later click, or reopen a review after the user presses Done, because every response unconditionally calls setActiveReview. Tracking a request generation and invalidating it from hideReview would ensure only the latest request can commit state.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/document-edit-review-context.tsx, line 65:

<comment>A slower earlier review request can replace the review selected by a later click, or reopen a review after the user presses Done, because every response unconditionally calls `setActiveReview`. Tracking a request generation and invalidating it from `hideReview` would ensure only the latest request can commit state.</comment>

<file context>
@@ -0,0 +1,96 @@
+				return;
+			}
+
+			setActiveReview({
+				beforeDocument: review.beforeDocument,
+				itemId: input.itemId,
</file context>

[cubic-dev-ai]

P2: A delayed undo response can close the wrong document's review. Make the hide conditional on the active review still matching this undo target (or use a review token) so switching receipts while the request is pending does not discard the new review.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/use-document-edit-receipt-undo.ts, line 24:

<comment>A delayed undo response can close the wrong document's review. Make the hide conditional on the active review still matching this undo target (or use a review token) so switching receipts while the request is pending does not discard the new review.</comment>

<file context>
@@ -0,0 +1,44 @@
+			// Every outcome ends the review: it either just undid the changes, or
+			// told us they no longer describe the document. Leaving the marks up
+			// after that would be showing a diff we have just been told is wrong.
+			hideReview();
+
+			if (result.status === "reverted") {
</file context>

[cubic-dev-ai]

P3: While a review is active the document is read-only, but it is still collaborative — any remote teammate typing arrives as a docChanged transaction, and the plugin's apply recomputes a full-document ChangeSet diff (plus a per-change document.forEach in addChangedAtomDecorations) on every such transaction. For a large or actively-edited document this is a meaningful recompute repeated for the whole duration of the review. Consider caching/recomputing the decorations only when the review is shown (or at most debounced/throttled), since interacting with the review itself doesn't change the doc while it is read-only.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/document-edit-review-extension.ts, line 76:

<comment>While a review is active the document is read-only, but it is still collaborative — any remote teammate typing arrives as a `docChanged` transaction, and the plugin's `apply` recomputes a full-document `ChangeSet` diff (plus a per-change `document.forEach` in `addChangedAtomDecorations`) on every such transaction. For a large or actively-edited document this is a meaningful recompute repeated for the whole duration of the review. Consider caching/recomputing the decorations only when the review is shown (or at most debounced/throttled), since interacting with the review itself doesn't change the doc while it is read-only.</comment>

<file context>
@@ -0,0 +1,204 @@
+
+						return {
+							beforeDocument,
+							decorations: createDocumentEditReviewDecorations(beforeDocument, newState.doc),
+						};
+					},
</file context>

[cubic-dev-ai]

9 issues found and verified against the latest diff

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/features/workspaces/ai/workspace-tools.ts">

<violation number="1" location="src/features/workspaces/ai/workspace-tools.ts:21">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The new `resolveWorkspaceReferences` capability is threaded through `createAIThreadWorkspaceTools` → `createWorkspaceThreadTool` → `createThreadWorkspaceAccessContext`, but no tests exercise this new path. The PR introduces a new reference-resolution behavior that should be validated by tests — for example, verifying that the callback is forwarded to `createWorkspaceAccessContext` when provided and omitted when absent. Adding coverage for this path would protect against future regressions where the wiring is accidentally dropped.</violation>
</file>

<file name="src/features/workspaces/model/workspace-ai-context-prompt.ts">

<violation number="1" location="src/features/workspaces/model/workspace-ai-context-prompt.ts:78">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The new empty-workspace prompt path is not covered by tests. Under the "Flag AI Slop and Fabricated Changes" rule, behavior-change PRs should exercise the changed behavior when a practical regression assertion exists. Add a test that passes a snapshot with `outline.totalItems === 0` and asserts the prompt includes the "this workspace is empty" message.</violation>
</file>

<file name="src/features/workspaces/operations/document-citations.ts">

<violation number="1" location="src/features/workspaces/operations/document-citations.ts:15">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The new `resolveDocumentCitations` function introduces document-level citation resolution behavior (ref→label mapping, PDF page-number formatting, and HTML re-injection), but no tests in the diff or repository exercise it. Rule 1 flags behavior-change code that lacks tests when regression-style assertions are practical, which is the case here: mocked `resolveWorkspaceReferences` and `getItemPaths` inputs would readily let you assert that a `wr_` ref maps to `"Document Name, p. 3"` for a PDF page, or plain `"Document Name"` otherwise. Adding unit tests for this path would protect against silent regressions in citation formatting and unknown-item filtering.</violation>
</file>

<file name="src/features/workspaces/components/WorkspaceLayout.tsx">

<violation number="1" location="src/features/workspaces/components/WorkspaceLayout.tsx:309">
P2: Switching between cached workspaces can retain the previous document's review state and resurrect its review when the user returns, because this stateful provider is not scoped to the current `workspaceId`. Remount the provider per workspace or clear `activeReview` whenever the workspace changes.</violation>
</file>

<file name="src/features/workspaces/components/ai-chat/AiChatDocumentEditActions.tsx">

<violation number="1" location="src/features/workspaces/components/ai-chat/AiChatDocumentEditActions.tsx:60">
P2: Reviewing a document fails after more than eight edits in one AI turn because this sends the entire unbounded receipt list while older receipts have already been evicted. The receipt group needs a retention/size guarantee shared with the server, or the UI needs to avoid presenting a review action that cannot be resolved.</violation>
</file>

<file name="src/features/workspaces/documents/document-edit-review-context.tsx">

<violation number="1" location="src/features/workspaces/documents/document-edit-review-context.tsx:65">
P2: A slower review request can overwrite a newer document selection, leaving the wrong document's changes active when users click multiple receipt rows before the first request finishes. Guard the response with a request-generation/token check (and invalidate it when hiding or changing the selection) before calling `setActiveReview`.</violation>
</file>

<file name="src/features/workspaces/documents/use-document-edit-receipt-undo.ts">

<violation number="1" location="src/features/workspaces/documents/use-document-edit-receipt-undo.ts:24">
P2: Opening another document’s review while an undo is pending can have that later review disappear when the earlier request completes, because this callback hides whichever review is currently active rather than the review being undone. Scoping the hide operation to the mutation’s `itemId` and `receiptIds` would preserve newer reviews.</violation>

<violation number="2" location="src/features/workspaces/documents/use-document-edit-receipt-undo.ts:26">
P2: A repeated or concurrent undo shows “Changes undone.” even when no changes were undone, because the server uses `status: "reverted"` for both successful and already-undone receipts. Distinguishing those outcomes in the undo result would let this hook show the intended “already undone” message.</violation>
</file>

<file name="src/features/workspaces/documents/document-ai-edits.ts">

<violation number="1" location="src/features/workspaces/documents/document-ai-edits.ts:181">
P2: Receipt line tallies overcount unchanged horizontal rules and block-math blocks after a whole-document rewrite. Exclude the top-level AI ref from the atom JSON before using it as the line key.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

The new resolveWorkspaceReferences capability is threaded through createAIThreadWorkspaceTools → createWorkspaceThreadTool → createThreadWorkspaceAccessContext, but no tests exercise this new path. The PR introduces a new reference-resolution behavior that should be validated by tests — for example, verifying that the callback is forwarded to createWorkspaceAccessContext when provided and omitted when absent. Adding coverage for this path would protect against future regressions where the wiring is accidentally dropped.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/ai/workspace-tools.ts, line 21:

<comment>The new `resolveWorkspaceReferences` capability is threaded through `createAIThreadWorkspaceTools` → `createWorkspaceThreadTool` → `createThreadWorkspaceAccessContext`, but no tests exercise this new path. The PR introduces a new reference-resolution behavior that should be validated by tests — for example, verifying that the callback is forwarded to `createWorkspaceAccessContext` when provided and omitted when absent. Adding coverage for this path would protect against future regressions where the wiring is accidentally dropped.</comment>

<file context>
@@ -18,6 +18,7 @@ type WorkspaceThreadToolConfig = {
 	definition: (typeof workspaceToolDefinitions)[number];
 	getThreadContext: () => Promise<AIThreadContext | null>;
 	onWorkspaceReferences?: (records: readonly WorkspaceReferenceRecord[]) => void;
+	resolveWorkspaceReferences?: (refs: readonly string[]) => Promise<WorkspaceReferenceRecord[]>;
 };
 
</file context>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

The new empty-workspace prompt path is not covered by tests. Under the "Flag AI Slop and Fabricated Changes" rule, behavior-change PRs should exercise the changed behavior when a practical regression assertion exists. Add a test that passes a snapshot with outline.totalItems === 0 and asserts the prompt includes the "this workspace is empty" message.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/model/workspace-ai-context-prompt.ts, line 78:

<comment>The new empty-workspace prompt path is not covered by tests. Under the "Flag AI Slop and Fabricated Changes" rule, behavior-change PRs should exercise the changed behavior when a practical regression assertion exists. Add a test that passes a snapshot with `outline.totalItems === 0` and asserts the prompt includes the "this workspace is empty" message.</comment>

<file context>
@@ -73,6 +73,12 @@ export function formatWorkspaceAiContextForPrompt(value: unknown) {
 function formatWorkspaceAiContextOutline(outline: WorkspaceAiContextOutline) {
+	// Said plainly. Reporting "0 items complete" buries an empty workspace in a
+	// sentence whose grammar reads as a truncation notice.
+	if (outline.totalItems === 0) {
+		return ["- Workspace outline: this workspace is empty. It has no items yet."];
+	}
</file context>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

The new resolveDocumentCitations function introduces document-level citation resolution behavior (ref→label mapping, PDF page-number formatting, and HTML re-injection), but no tests in the diff or repository exercise it. Rule 1 flags behavior-change code that lacks tests when regression-style assertions are practical, which is the case here: mocked resolveWorkspaceReferences and getItemPaths inputs would readily let you assert that a wr_ ref maps to "Document Name, p. 3" for a PDF page, or plain "Document Name" otherwise. Adding unit tests for this path would protect against silent regressions in citation formatting and unknown-item filtering.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/operations/document-citations.ts, line 17:

<comment>The new `resolveDocumentCitations` function introduces document-level citation resolution behavior (ref→label mapping, PDF page-number formatting, and HTML re-injection), but no tests in the diff or repository exercise it. Rule 1 flags behavior-change code that lacks tests when regression-style assertions are practical, which is the case here: mocked `resolveWorkspaceReferences` and `getItemPaths` inputs would readily let you assert that a `wr_` ref maps to `"Document Name, p. 3"` for a PDF page, or plain `"Document Name"` otherwise. Adding unit tests for this path would protect against silent regressions in citation formatting and unknown-item filtering.</comment>

<file context>
@@ -0,0 +1,60 @@
+ * lets the document store the item and page it points at, named as the reader
+ * knows it.
+ */
+export async function resolveDocumentCitations(input: {
+	context: WorkspaceAccessContext;
+	html: string;
</file context>

[cubic-dev-ai]

P2: Switching between cached workspaces can retain the previous document's review state and resurrect its review when the user returns, because this stateful provider is not scoped to the current workspaceId. Remount the provider per workspace or clear activeReview whenever the workspace changes.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/WorkspaceLayout.tsx, line 309:

<comment>Switching between cached workspaces can retain the previous document's review state and resurrect its review when the user returns, because this stateful provider is not scoped to the current `workspaceId`. Remount the provider per workspace or clear `activeReview` whenever the workspace changes.</comment>

<file context>
@@ -305,11 +306,13 @@ export function WorkspaceShell({
-				) : (
-					workspaceInteractionContent
-				)}
+				<DocumentEditReviewProvider workspaceId={workspace.id}>
+					{hasHeavyViewerRuntimeItems ? (
+						<WorkspacePdfEngineProvider>{workspaceInteractionContent}</WorkspacePdfEngineProvider>
</file context>

Suggested change

	<DocumentEditReviewProvider workspaceId={workspace.id}>
	<DocumentEditReviewProvider key={workspace.id} workspaceId={workspace.id}>

[cubic-dev-ai]

P2: Reviewing a document fails after more than eight edits in one AI turn because this sends the entire unbounded receipt list while older receipts have already been evicted. The receipt group needs a retention/size guarantee shared with the server, or the UI needs to avoid presenting a review action that cannot be resolved.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/ai-chat/AiChatDocumentEditActions.tsx, line 60:

<comment>Reviewing a document fails after more than eight edits in one AI turn because this sends the entire unbounded receipt list while older receipts have already been evicted. The receipt group needs a retention/size guarantee shared with the server, or the UI needs to avoid presenting a review action that cannot be resolved.</comment>

<file context>
@@ -0,0 +1,104 @@
+			type="button"
+			className="flex w-full min-w-0 items-center gap-2 px-2.5 py-2 text-left transition-colors hover:bg-foreground/5"
+			onClick={() => {
+				void showReview({ itemId: group.itemId, receiptIds: group.receiptIds });
+			}}
+		>
</file context>

[cubic-dev-ai]

P2: A slower review request can overwrite a newer document selection, leaving the wrong document's changes active when users click multiple receipt rows before the first request finishes. Guard the response with a request-generation/token check (and invalidate it when hiding or changing the selection) before calling setActiveReview.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/document-edit-review-context.tsx, line 65:

<comment>A slower review request can overwrite a newer document selection, leaving the wrong document's changes active when users click multiple receipt rows before the first request finishes. Guard the response with a request-generation/token check (and invalidate it when hiding or changing the selection) before calling `setActiveReview`.</comment>

<file context>
@@ -0,0 +1,96 @@
+				return;
+			}
+
+			setActiveReview({
+				beforeDocument: review.beforeDocument,
+				itemId: input.itemId,
</file context>

[cubic-dev-ai]

P2: A repeated or concurrent undo shows “Changes undone.” even when no changes were undone, because the server uses status: "reverted" for both successful and already-undone receipts. Distinguishing those outcomes in the undo result would let this hook show the intended “already undone” message.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/use-document-edit-receipt-undo.ts, line 26:

<comment>A repeated or concurrent undo shows “Changes undone.” even when no changes were undone, because the server uses `status: "reverted"` for both successful and already-undone receipts. Distinguishing those outcomes in the undo result would let this hook show the intended “already undone” message.</comment>

<file context>
@@ -0,0 +1,44 @@
+			// after that would be showing a diff we have just been told is wrong.
+			hideReview();
+
+			if (result.status === "reverted") {
+				toast.success("Changes undone.");
+			} else {
</file context>

[cubic-dev-ai]

P2: Opening another document’s review while an undo is pending can have that later review disappear when the earlier request completes, because this callback hides whichever review is currently active rather than the review being undone. Scoping the hide operation to the mutation’s itemId and receiptIds would preserve newer reviews.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/use-document-edit-receipt-undo.ts, line 24:

<comment>Opening another document’s review while an undo is pending can have that later review disappear when the earlier request completes, because this callback hides whichever review is currently active rather than the review being undone. Scoping the hide operation to the mutation’s `itemId` and `receiptIds` would preserve newer reviews.</comment>

<file context>
@@ -0,0 +1,44 @@
+			// Every outcome ends the review: it either just undid the changes, or
+			// told us they no longer describe the document. Leaving the marks up
+			// after that would be showing a diff we have just been told is wrong.
+			hideReview();
+
+			if (result.status === "reverted") {
</file context>

[cubic-dev-ai]

P2: Receipt line tallies overcount unchanged horizontal rules and block-math blocks after a whole-document rewrite. Exclude the top-level AI ref from the atom JSON before using it as the line key.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/documents/document-ai-edits.ts, line 181:

<comment>Receipt line tallies overcount unchanged horizontal rules and block-math blocks after a whole-document rewrite. Exclude the top-level AI ref from the atom JSON before using it as the line key.</comment>

<file context>
@@ -0,0 +1,312 @@
+			}
+			// A rule or a formula holds no text but still occupies a line.
+			if (node.isAtom) {
+				countLine(JSON.stringify(node.toJSON()));
+				return false;
+			}
</file context>