Refactor workspace content architecture#664
Conversation
|
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. |
|
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. |
|
React Doctor found 3 new issues in 3 files · 3 warnings · score 87 / 100 (Great) · 2 fixed · vs 3 warnings
Reviewed by React Doctor for commit |
📝 WalkthroughWalkthroughWorkspace reads now use typed, paginated contracts with document/file continuation, revision checks, and relation paths. Workspace operations use authorized kernel path resolution, while kernel events, projections, realtime state, caching, and tool receipts carry updated item facts and pending statuses. ChangesWorkspace content reads
Kernel and operation flow
Workspace state and tooling
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
|
|
Preview deployment for your docs. Learn more about Mintlify Previews.
|
Greptile SummaryThis PR refactors workspace content reads and item operations around the kernel seam. The main changes are:
Confidence Score: 4/5Mostly safe to merge after fixing the batch-create relation bug. One contained issue remains in ordered batch item creation with relations. The main read-path and projection changes are isolated behind new seams and covered by focused tests.
What T-Rex did
Important Files Changed
Reviews (1): Last reviewed commit: "docs(workspaces): shape document AI edit..." | Re-trigger Greptile |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d1d6686a53
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
9f18199 to
70a2e49
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/features/workspaces/realtime/messages.ts (1)
20-46: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftNormalize pre-refactor events before replay.
Persisted created/deleted events lack
itemFacts; replaying them into the new page reducer can call.maponundefined. Migrate or normalize historical events, or force a fresh snapshot when an old payload is encountered.🤖 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/realtime/messages.ts` around lines 20 - 46, Update the realtime event replay path for historical workspace item created/deleted events so payloads missing itemFacts are migrated or normalized before reaching the page reducer. Ensure the reducer always receives an itemFacts array, or trigger a fresh snapshot when normalization is impossible; preserve current handling for events that already include itemFacts. Reference the created/deleted event handling and the page-reducer replay entry point.
🤖 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/content/workspace-content-reader.ts`:
- Around line 249-265: The typed content readers must preserve Markdown
whitespace exactly. In createDocumentChunk, remove trimEnd() from the source
slice while keeping cursor advancement based on the original end boundary; in
workspace-page-projection.ts lines 189-198, return page Markdown without trim()
and avoid trimming during assembly.
In `@src/features/workspaces/documents/document-session.ts`:
- Around line 179-187: Update readMarkdown to serialize the current Tiptap
document into a local Markdown value before awaiting crypto.subtle.digest, then
return that captured value with the corresponding revision. Keep the
state-vector digest flow unchanged while ensuring both fields represent the same
document snapshot.
In `@src/features/workspaces/model/workspace-page.ts`:
- Around line 47-61: Update upsertWorkspaceItemFactsInPage to return the
existing page unchanged when the incoming revision is lower than page.revision,
before merging itemFacts. Preserve the current merge behavior and
maximum-revision handling for revisions equal to or newer than the page
revision.
In `@src/features/workspaces/operations/workspace-operation-observability.ts`:
- Around line 57-70: Update summarizeWorkspaceReadResult in
src/features/workspaces/operations/workspace-operation-observability.ts:57-70 to
count only ready results as succeeded and represent pending results explicitly
in the summary. In
src/features/workspaces/components/ai-chat/ai-chat-tool-receipts.ts:256-260,
update the read-result handling to detect pending results and avoid returning
completed("Read 0 items").
---
Outside diff comments:
In `@src/features/workspaces/realtime/messages.ts`:
- Around line 20-46: Update the realtime event replay path for historical
workspace item created/deleted events so payloads missing itemFacts are migrated
or normalized before reaching the page reducer. Ensure the reducer always
receives an itemFacts array, or trigger a fresh snapshot when normalization is
impossible; preserve current handling for events that already include itemFacts.
Reference the created/deleted event handling and the page-reducer replay entry
point.
🪄 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
Run ID: 9fcccb10-69b0-4628-9933-29327ffff603
📒 Files selected for processing (46)
docs/project/document-ai-editing-refactor.mdsrc/features/workspaces/ai/ai-thread-runtime.tssrc/features/workspaces/cache-workspace.tssrc/features/workspaces/components/WorkspaceCreateMenu.tsxsrc/features/workspaces/components/ai-chat/ai-chat-tool-receipts.tssrc/features/workspaces/content/workspace-content-contract.test.tssrc/features/workspaces/content/workspace-content-contract.tssrc/features/workspaces/content/workspace-content-cursor.tssrc/features/workspaces/content/workspace-content-reader.test.tssrc/features/workspaces/content/workspace-content-reader.tssrc/features/workspaces/contracts.tssrc/features/workspaces/defaults.tssrc/features/workspaces/document-session-access.tssrc/features/workspaces/documents/document-item-content.tssrc/features/workspaces/documents/document-session.tssrc/features/workspaces/extraction/workspace-page-projection.test.tssrc/features/workspaces/extraction/workspace-page-projection.tssrc/features/workspaces/kernel/workspace-kernel-access.tssrc/features/workspaces/kernel/workspace-kernel-events.test.tssrc/features/workspaces/kernel/workspace-kernel-file-commands.tssrc/features/workspaces/kernel/workspace-kernel-files.tssrc/features/workspaces/kernel/workspace-kernel-item-commands.tssrc/features/workspaces/kernel/workspace-kernel-relations.tssrc/features/workspaces/kernel/workspace-kernel-store.tssrc/features/workspaces/kernel/workspace-kernel-types.tssrc/features/workspaces/kernel/workspace-kernel.tssrc/features/workspaces/model/item-display.tssrc/features/workspaces/model/object-registry.tssrc/features/workspaces/model/workspace-item-colors.tssrc/features/workspaces/model/workspace-page.test.tssrc/features/workspaces/model/workspace-page.tssrc/features/workspaces/operations/create-items.tssrc/features/workspaces/operations/delete-items.tssrc/features/workspaces/operations/edit-item.tssrc/features/workspaces/operations/link-items.tssrc/features/workspaces/operations/list-items.tssrc/features/workspaces/operations/move-items.tssrc/features/workspaces/operations/read-items.tssrc/features/workspaces/operations/relations.tssrc/features/workspaces/operations/rename-item.tssrc/features/workspaces/operations/workspace-operation-context.tssrc/features/workspaces/operations/workspace-operation-observability.tssrc/features/workspaces/operations/workspace-tool-definitions.tssrc/features/workspaces/operations/workspace-tool-schemas.tssrc/features/workspaces/realtime/messages.tssrc/features/workspaces/use-create-workspace.ts
💤 Files with no reviewable changes (3)
- src/features/workspaces/defaults.ts
- src/features/workspaces/model/workspace-item-colors.ts
- src/features/workspaces/kernel/workspace-kernel-store.ts
There was a problem hiding this comment.
Review completed against the latest diff
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Consume page bodies without retaining live R2 responses. Preserve exact Markdown and keep old manifests readable under the runtime byte budget. Move document chunking behind DocumentSession with a revision-keyed snapshot cache. Make projection updates idempotent across workflow retries. Tests: pnpm test; pnpm test:workers; pnpm exec vp check
Validate realtime messages before cache updates and collapse reconnect, revision-gap, and invalid-message recovery into one canonical refetch path. Ignore stale events, query facts only for selected items, and report pending extraction separately from successful reads. Tests: pnpm test; pnpm test:workers; pnpm exec vp check
Run an explicit Durable Object SQLite migration before current item schemas load. Remove legacy relations and projections, then soft-retire flashcard and quiz rows without a runtime shim. Tests: pnpm test; pnpm test:workers; pnpm exec vp check
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/features/workspaces/extraction/workspace-page-projection.ts (2)
342-350: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep whitespace, but do not treat whitespace-only extraction as usable.
Returning raw Markdown is correct, but
writeWorkspacePageProjectionsubsequently checkspage.markdown.length > 0. A page containing only spaces/newlines can therefore publish an otherwise unusable projection. Usepage.markdown.trim().length > 0solely for the usability check while preserving the stored value.🤖 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/extraction/workspace-page-projection.ts` around lines 342 - 350, Update writeWorkspacePageProjection’s Markdown usability check to use page.markdown.trim().length > 0, so whitespace-only extraction is rejected. Preserve normalizeProjectionPage’s raw Markdown return value and retain the original whitespace when storing usable page content.
11-11: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not downgrade the persisted projection schema version.
Existing version-2 manifests now fail the exact schema check at Lines 249-250. Keep version 2 while making
pagesoptional, or explicitly support both versions during migration.-const projectionSchemaVersion = 1; +const projectionSchemaVersion = 2;🤖 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/extraction/workspace-page-projection.ts` at line 11, Update projectionSchemaVersion to preserve version 2 rather than downgrading persisted manifests. Keep the pages field optional while retaining version-2 compatibility, or explicitly accept both schema versions in the validation and migration logic.
🧹 Nitpick comments (1)
src/features/workspaces/kernel/workspace-kernel-item-commands.ts (1)
388-404: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove unreachable dead code.
Because the method immediately throws an error if
type !== "document"at line 378, theelseblock here is completely unreachable.♻️ Proposed refactor
- if (type === "document") { - persistDocumentItemContentUpdate({ - content: input.content, - itemId: input.itemId, - metadataJson: item.metadata_json, - sql: this.sql, - updatedAt: now, - }); - } else { - touchWorkspaceItemUpdatedAt({ - itemId: input.itemId, - sql: this.sql, - updatedAt: now, - }); - } + persistDocumentItemContentUpdate({ + content: input.content, + itemId: input.itemId, + metadataJson: item.metadata_json, + sql: this.sql, + updatedAt: now, + });🤖 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/kernel/workspace-kernel-item-commands.ts` around lines 388 - 404, Remove the unreachable else branch calling touchWorkspaceItemUpdatedAt from the update method, since the earlier type validation guarantees type is "document". Keep the persistDocumentItemContentUpdate call and its existing arguments unchanged.
🤖 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/documents/document-markdown-chunk.ts`:
- Around line 37-43: Adjust the chunk-boundary calculation around hardEnd in the
document markdown chunking logic to back off one UTF-16 code unit when it falls
between a high and low surrogate, ensuring content never splits a surrogate
pair. Add a boundary test covering mixed ASCII and emoji text while preserving
the existing newline and minimum-chunk behavior.
In `@src/features/workspaces/kernel/workspace-kernel-item-commands.ts`:
- Around line 81-96: Update the prior-result lookup in getPriorResult to run
whenever clientMutationId is provided, including requests without input.id.
Modify findCreatedItemEvent to identify the created item from the event payload
and return its itemId, then use that returned ID to fetch the item while
preserving the existing hydration and result structure.
In `@src/features/workspaces/realtime/use-workspace-presence.ts`:
- Around line 130-133: Update the message-handling branch around
parseServerMessage so a null or unrecognized message is ignored and returned
without invoking onDesyncRef.current. Preserve desync handling for actual
synchronization failures detected elsewhere in the workspace presence flow.
---
Outside diff comments:
In `@src/features/workspaces/extraction/workspace-page-projection.ts`:
- Around line 342-350: Update writeWorkspacePageProjection’s Markdown usability
check to use page.markdown.trim().length > 0, so whitespace-only extraction is
rejected. Preserve normalizeProjectionPage’s raw Markdown return value and
retain the original whitespace when storing usable page content.
- Line 11: Update projectionSchemaVersion to preserve version 2 rather than
downgrading persisted manifests. Keep the pages field optional while retaining
version-2 compatibility, or explicitly accept both schema versions in the
validation and migration logic.
---
Nitpick comments:
In `@src/features/workspaces/kernel/workspace-kernel-item-commands.ts`:
- Around line 388-404: Remove the unreachable else branch calling
touchWorkspaceItemUpdatedAt from the update method, since the earlier type
validation guarantees type is "document". Keep the
persistDocumentItemContentUpdate call and its existing arguments unchanged.
🪄 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
Run ID: 5df82d2f-dbd7-4965-8f9e-f071c948175f
📒 Files selected for processing (30)
src/features/workspaces/components/WorkspaceLayout.tsxsrc/features/workspaces/components/ai-chat/ai-chat-tool-receipts.test.tssrc/features/workspaces/components/ai-chat/ai-chat-tool-receipts.tssrc/features/workspaces/content/workspace-content-contract.tssrc/features/workspaces/content/workspace-content-reader.test.tssrc/features/workspaces/content/workspace-content-reader.tssrc/features/workspaces/document-session-access.tssrc/features/workspaces/documents/document-markdown-chunk.test.tssrc/features/workspaces/documents/document-markdown-chunk.tssrc/features/workspaces/documents/document-session.tssrc/features/workspaces/extraction/liteparse-projection.tssrc/features/workspaces/extraction/request-workspace-file-extraction.tssrc/features/workspaces/extraction/workspace-file-extraction-workflow.tssrc/features/workspaces/extraction/workspace-page-projection.test.tssrc/features/workspaces/extraction/workspace-page-projection.tssrc/features/workspaces/kernel/workspace-kernel-events.test.tssrc/features/workspaces/kernel/workspace-kernel-events.tssrc/features/workspaces/kernel/workspace-kernel-file-commands.tssrc/features/workspaces/kernel/workspace-kernel-item-commands.tssrc/features/workspaces/kernel/workspace-kernel-list.tssrc/features/workspaces/kernel/workspace-kernel-rows.tssrc/features/workspaces/kernel/workspace-kernel-store.tssrc/features/workspaces/kernel/workspace-kernel-types.tssrc/features/workspaces/kernel/workspace-kernel.tssrc/features/workspaces/model/workspace-page.tssrc/features/workspaces/operations/create-items.tssrc/features/workspaces/operations/workspace-operation-observability.test.tssrc/features/workspaces/operations/workspace-operation-observability.tssrc/features/workspaces/realtime/messages.tssrc/features/workspaces/realtime/use-workspace-presence.ts
💤 Files with no reviewable changes (1)
- src/features/workspaces/kernel/workspace-kernel-types.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/features/workspaces/kernel/workspace-kernel-file-commands.ts
- src/features/workspaces/operations/create-items.ts
- src/features/workspaces/model/workspace-page.ts
- src/features/workspaces/content/workspace-content-contract.ts
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: Auto-approval blocked by 3 unresolved issues from previous reviews.
Re-trigger cubic
Keep flashcards, quizzes, and audio recording as future workspace capabilities. Remove the speculative retirement migration. Use explicit menu groups instead of generic registry filters.
Use the generated DocumentSession stub instead of a hand-written client mirror. Keep the narrow read seam at its actual test boundary. Hide kernel list selection and formatting behind one complete operation.
There was a problem hiding this comment.
All reported issues were addressed across 14 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Keep document chunks on valid UTF-16 boundaries and ignore unknown realtime messages. Require stable item IDs for idempotent create commands.
There was a problem hiding this comment.
All reported issues were addressed across 6 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Use one Base64URL codec and SHA-256 helper. Share it across upload tokens, content cursors, extraction identities, and document revisions.
Define item schemas, names, labels, colors, and storage metadata in one server-safe registry. Keep React icons in the UI adapter.
Keep realtime event payloads immutable. Store idempotency identity in a dedicated receipt table keyed by client mutation ID.
Reuse the canonical create command input so kernel clients cannot omit stable item IDs.
There was a problem hiding this comment.
0 issues found across 24 files (changes from recent commits).
Requires human review: Requires human sign-off for contract change making item creation require stable IDs, and for merging reconnect and desync recovery handlers, which alters client cache invalidation behavior.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: Changes workspace_read_items tool input/output contract (new required mode field, output shape) and makes item creation id required. Contract changes need human verification for backward compatibility and consumer updates.
Re-trigger cubic
Summary
Why
The old read path fetched every selected R2 page before consuming any response body. That retained all bodies at once and made resource use scale with page count. Cloudflare changed connection accounting on April 9, 2026, so the historical connection-limit exception is not proven on the current runtime, but the fan-out remains an unnecessary resource pattern.
Workspace semantics were also duplicated across operations, document reads transferred more state than necessary, and several internal interfaces exposed intermediate plumbing rather than a complete operation.
The model-facing contract stays compact: start a path, select physical pages when useful, and continue with an opaque cursor. Storage identity, linking, revision checks, byte budgets, and extraction lifecycle remain internal.
Changes
Testing
Review Notes
Start with workspace-content-reader.ts, workspace-page-projection.ts, workspace-item-registry.ts, workspace-kernel-events.ts, and workspace-kernel-schema.ts. The sequential read loops are deliberate resource boundaries. No planning Markdown document is included.