packages/backend/prisma/schema.prisma (1)

35-35: LGTM!

The type field with @default("sheet") ensures backward compatibility for existing documents.

Consider using a Prisma enum instead of String if the valid types ("sheet", "doc") are fixed:

♻️ Optional: Use enum for type safety

+enum DocumentType {
+  sheet
+  doc
+}
+
 model Document {
   id        String   `@default`(uuid()) `@id`
   title     String
-  type      String   `@default`("sheet")
+  type      DocumentType `@default`(sheet)
   ...
 }

This provides database-level validation and better type safety in generated Prisma Client code.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/backend/prisma/schema.prisma` at line 35, Replace the String-typed
"type" field with a Prisma enum to enforce allowed values and DB-level
validation: add an enum (e.g., DocumentType with members sheet and doc), change
the "type" field's type from String to that enum (e.g., DocumentType) and set
its `@default` to the enum member (e.g., sheet), then run Prisma migrate/generate
so Prisma Client types are updated; target the "type" field and the schema's
enums when making the change.

packages/frontend/vite.config.ts (1)

60-80: Consider adding chunk configuration for docs package.

The manualChunks function has dedicated chunks for sheet-formula-parser, sheet-formula-eval, and sheet-core. The new @wafflebase/docs package doesn't have similar chunking rules, so its code will be bundled into the main application chunk. If the docs package is substantial, you may want to add a docs-core chunk for better caching and parallel loading.

💡 Optional: Add docs package chunking

   if (
     normalizedId.includes("/packages/sheets/src/view/") ||
     normalizedId.includes("/packages/sheets/src/model/") ||
     normalizedId.includes("/packages/sheets/src/store/")
   ) {
     return "sheet-core";
   }

+  if (normalizedId.includes("/packages/docs/src/")) {
+    return "docs-core";
+  }
+
   return undefined;
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/frontend/vite.config.ts` around lines 60 - 80, Add a manualChunks
rule to split the `@wafflebase/docs` package into its own chunk: detect
normalizedId.includes("@wafflebase/docs") (or the package path under
/packages/docs/) in the same manualChunks function that currently returns
"sheet-formula-parser", "sheet-formula-eval", and "sheet-core", and return a new
chunk name like "docs-core" so docs code is bundled separately for better
caching and parallel loading.

packages/frontend/src/api/workspaces.ts (1)

227-238: Consider adding return type annotation for consistency.

Other workspace functions like createWorkspace() and updateWorkspace() have explicit return type annotations (e.g., Promise<Workspace>). This function lacks one, which reduces type safety at call sites.

♻️ Proposed fix

+import type { Document } from "@/types/documents";
+
 export async function createWorkspaceDocument(
   workspaceId: string,
   data: { title: string; type?: "sheet" | "doc" },
-) {
+): Promise<Document> {
   const res = await fetchWithAuth(`${BASE}/${workspaceId}/documents`, {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/frontend/src/api/workspaces.ts` around lines 227 - 238,
createWorkspaceDocument lacks an explicit return type which reduces type safety;
update the function signature for createWorkspaceDocument to include an explicit
Promise-return type (e.g., Promise<Document> or the project-specific document
type you use) and ensure that type is imported or defined where necessary so the
returned res.json() result matches the annotated type.

packages/backend/src/api/v1/documents.controller.ts (1)

34-38: Consider stricter type validation or a comment explaining the default behavior.

Using type?: string allows any string value, which is then silently converted to 'sheet' if not exactly 'doc'. This is a valid fail-safe approach, but consider either:

1. Adding a brief comment explaining the intentional defaulting, or
2. Using a DTO with validation (e.g., class-validator) to reject invalid type values

♻️ Option 1: Add clarifying comment

   `@Post`()
   async create(
     `@Param`('workspaceId') workspaceId: string,
     `@Req`() req: AuthenticatedRequest,
     `@Body`() body: { title: string; type?: string },
   ) {
     return this.documentService.createDocument({
       title: body.title,
+      // Default to 'sheet' for missing or unrecognized type values
       type: body.type === 'doc' ? 'doc' : 'sheet',

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/backend/src/api/v1/documents.controller.ts` around lines 34 - 38,
The current controller accepts `@Body`() body: { title: string; type?: string }
and maps body.type === 'doc' ? 'doc' : 'sheet' which silently coerces any
invalid string to 'sheet'; update this by either (A) replacing the loose inline
type with a dedicated DTO (e.g., DocumentCreateDto) that declares type: 'doc' |
'sheet' and add class-validator decorators to enforce allowed values before
calling documentService.createDocument, or (B) at minimum add a clarifying
comment next to the mapping (in the create handler) explaining the intentional
default-to-'sheet' behavior and consider narrowing the parameter type to type?:
'doc' | 'sheet' so invalid values are flagged by TypeScript; reference
body.type, the controller create handler, and documentService.createDocument
when making the change.

packages/frontend/src/app/shared/shared-document.tsx (1)

194-202: Type annotation should use Partial<YorkieDocsRoot> for consistency.

The YorkieDocsRoot type requires a content: Tree property, but initialRoot={{}} provides an empty object. While this works at runtime (Yorkie initializes the Tree lazily), the type annotation creates a mismatch. The pattern in docs-detail.tsx (context snippet 3) correctly uses Partial<YorkieDocsRoot> for this case.

♻️ Proposed fix

       {isDocs ? (
-        <DocumentProvider<YorkieDocsRoot>
+        <DocumentProvider<Partial<YorkieDocsRoot>>
           docKey={docKey}
           initialRoot={{}}
           initialPresence={presence}
           enableDevtools={import.meta.env.DEV}
         >

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/frontend/src/app/shared/shared-document.tsx` around lines 194 - 202,
Change the generic type passed to DocumentProvider from YorkieDocsRoot to
Partial<YorkieDocsRoot> so the provided initialRoot={{}} matches the type;
update the DocumentProvider<...> usage in the component (the instance
surrounding initialRoot and presence props that renders SharedDocsLayout) to
DocumentProvider<Partial<YorkieDocsRoot>> to align with the pattern used in
docs-detail.tsx and avoid the required content: Tree property mismatch.

packages/frontend/src/app/documents/document-list.tsx (1)

295-323: Consider extracting the dropdown menu into a shared component.

The "New Sheet / New Document" dropdown menu is duplicated between the header area (lines 295-323) and the empty state (lines 378-408). Extracting this into a reusable component would reduce duplication and ensure consistent behavior.

♻️ Example extraction

function NewDocumentDropdown({ 
  onCreateSheet, 
  onCreateDoc,
  buttonSize = "default" 
}: { 
  onCreateSheet: () => void; 
  onCreateDoc: () => void;
  buttonSize?: "default" | "sm";
}) {
  return (
    <DropdownMenu>
      <DropdownMenuTrigger asChild>
        <Button size={buttonSize} className="flex items-center gap-2">
          <Plus className="w-4 h-4" />
          New
        </Button>
      </DropdownMenuTrigger>
      <DropdownMenuContent>
        <DropdownMenuItem onClick={onCreateSheet}>
          <Sheet className="mr-2 h-4 w-4 text-green-600" />
          New Sheet
        </DropdownMenuItem>
        <DropdownMenuItem onClick={onCreateDoc}>
          <FileText className="mr-2 h-4 w-4 text-blue-500" />
          New Document
        </DropdownMenuItem>
      </DropdownMenuContent>
    </DropdownMenu>
  );
}

Also applies to: 378-408

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/frontend/src/app/documents/document-list.tsx` around lines 295 -
323, Extract the duplicated "New" dropdown into a reusable component (e.g.,
NewDocumentDropdown) and replace both occurrences with it; implement
NewDocumentDropdown to accept onCreateSheet and onCreateDoc callbacks (and an
optional buttonSize prop) and internally render the DropdownMenu,
DropdownMenuTrigger, DropdownMenuContent, and two DropdownMenuItem entries with
the Sheet and FileText icons, then update the usages that currently call
createDocumentMutation.mutate(...) to pass wrapper callbacks like () =>
createDocumentMutation.mutate({ title: "New Sheet" }) and () =>
createDocumentMutation.mutate({ title: "New Document", type: "doc" }) so
behavior remains identical.

packages/frontend/src/app/docs/docs-formatting-toolbar.tsx (1)

117-171: Toggle buttons don't reflect current selection state.

The Toggle components use onPressedChange for toggling but don't set the pressed prop to reflect whether the current selection has that style applied. This means the buttons won't visually indicate when text is already bold, italic, etc.

♻️ Proposed fix to reflect active state

+  // Get current selection style for toggle states
+  const currentStyle = editor?.getSelectionStyle() ?? {};
+
   return (
     <div className="flex items-center gap-0.5 overflow-x-auto border-b bg-background px-2 py-1 whitespace-nowrap">
       {/* ... */}
       <Tooltip>
         <TooltipTrigger asChild>
           <Toggle
             size="sm"
+            pressed={!!currentStyle.bold}
             onPressedChange={toggleBold}
             className="h-7 w-7 cursor-pointer"
             aria-label="Bold"
           >
             <IconBold size={16} />
           </Toggle>
         </TooltipTrigger>
         <TooltipContent>Bold ({modKey}+B)</TooltipContent>
       </Tooltip>
       {/* Similar for italic, underline, strikethrough */}

Note: You may need to trigger re-renders when selection changes to keep the toggle states in sync.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/frontend/src/app/docs/docs-formatting-toolbar.tsx` around lines 117
- 171, The Toggle buttons in docs-formatting-toolbar.tsx must reflect the editor
selection state: add the pressed prop to each Toggle (the ones using toggleBold,
toggleItalic, toggleUnderline, toggleStrikethrough) and set it from the editor's
active state (e.g., editor.isActive('bold'), editor.isActive('italic'),
'underline', 'strike' or equivalent command names used by your editor instance).
Ensure the toolbar component subscribes to editor selection/transaction updates
(or uses a reactive hook from your editor wrapper) so the component re-renders
when selection/style state changes, keeping the pressed props in sync with the
current selection.

docs/tasks/active/20260323-docs-frontend-integration-todo.md (1)

1-25: Consider adding the companion lessons file.

Per coding guidelines, non-trivial task documentation should use paired files: YYYYMMDD-<slug>-todo.md and YYYYMMDD-<slug>-lessons.md. If there are any lessons learned from this integration (e.g., the Yorkie Tree re-export workaround, route naming due to proxy conflicts), consider documenting them in 20260323-docs-frontend-integration-lessons.md.

Based on learnings: "Non-trivial task documentation uses paired files in docs/tasks/active/: YYYYMMDD-<slug>-todo.md and YYYYMMDD-<slug>-lessons.md."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/tasks/active/20260323-docs-frontend-integration-todo.md` around lines 1
- 25, Add a companion lessons file named
20260323-docs-frontend-integration-lessons.md alongside the existing
20260323-docs-frontend-integration-todo.md; in that file summarize post-work
learnings (e.g., the Yorkie Tree re-export workaround, route naming/proxy
conflicts, verification notes such as the frontend test failure), reference the
spec docs/design/docs-frontend-integration.md, and follow the same
header/metadata conventions used by other paired task files in docs/tasks/active
so the two files are discoverable and consistent.

packages/frontend/src/app/docs/docs-detail.tsx (2)

106-113: Consider adding error handling for rename operation.

The handleRenameDocument function doesn't handle errors from renameDocument. If the API call fails, the user won't see feedback, and queries will still be invalidated.

♻️ Suggested improvement

   const handleRenameDocument = useCallback(
     async (newTitle: string) => {
+      try {
         await renameDocument(documentId, newTitle);
         queryClient.invalidateQueries({ queryKey: ["document", documentId] });
         queryClient.invalidateQueries({ queryKey: ["documents"] });
+      } catch (error) {
+        console.error("Failed to rename document:", error);
+        // Optionally show a toast notification to the user
+      }
     },
     [documentId, queryClient],
   );

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/frontend/src/app/docs/docs-detail.tsx` around lines 106 - 113, The
rename handler handleRenameDocument should catch errors from renameDocument and
avoid invalidating queries on failure; update handleRenameDocument to wrap the
async call in try/catch, call renameDocument(documentId, newTitle) inside try,
run queryClient.invalidateQueries(...) only on success, and in catch call an
error reporting UI/notification or processLogger (e.g., show toast or set an
error state) so the user gets feedback; ensure the function still rethrows or
returns a boolean if calling code depends on success/failure.

---

148-150: Add explicit guard for missing id parameter.

The id parameter from useParams is typed as string | undefined, but Line 185 uses a non-null assertion id!. While the render logic would show a loader indefinitely if id were undefined, an explicit early return with an error message would be clearer.

♻️ Suggested improvement

 export function DocsDetail() {
   const { id } = useParams();
+
+  if (!id) {
+    return (
+      <div className="flex h-full w-full items-center justify-center">
+        <div className="text-red-500">Document ID is required</div>
+      </div>
+    );
+  }

   const {
     data: currentUser,

Then Line 185 becomes safe without the assertion:

-      <DocsLayout documentId={id!} />
+      <DocsLayout documentId={id} />

Also applies to: 185-185

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/frontend/src/app/docs/docs-detail.tsx` around lines 148 - 150, The
component DocsDetail reads the route param id (const { id } = useParams()) but
later uses id! (on the value at the render/loader location around the current id
usage on line 185); add an explicit guard early in DocsDetail that checks if id
is undefined and immediately returns a clear error UI (or redirects) so the rest
of the component can assume a present id, and then remove the non-null assertion
(id!) wherever used (e.g., the usage around line 185) so code is safe and types
align.

packages/frontend/src/app/docs/yorkie-doc-store.ts (1)

292-303: Cache update pattern calls getDocument() unnecessarily.

After doc.update(), calling getDocument() rebuilds the document from the Yorkie tree. Since you already have the block to insert, you could update the cached document directly like updateBlock does.

♻️ Suggested optimization

   insertBlock(index: number, block: Block): void {
+    // Get current doc before mutation for cache update
+    const currentDoc = this.cachedDoc ? cloneDocument(this.cachedDoc) : this.getDocument();
     this.doc.update((root) => {
       const tree = root.content;
       if (!tree || typeof tree.getRootTreeNode !== 'function') return;
       tree.editByPath([index], [index], buildBlockNode(block));
     });
     // Update cache in-place
-    const currentDoc = this.getDocument();
     currentDoc.blocks.splice(index, 0, block);
     this.cachedDoc = currentDoc;
     this.dirty = false;
   }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/frontend/src/app/docs/yorkie-doc-store.ts` around lines 292 - 303,
insertBlock currently calls getDocument() after doc.update which forces a
rebuild; instead update the in-memory cache directly like updateBlock: ensure
this.cachedDoc exists (initialize from this.getDocument() only if null), then do
this.cachedDoc.blocks.splice(index, 0, block) and assign this.cachedDoc (or keep
same ref if intended), remove the local currentDoc variable and the extra
getDocument() call, and keep setting this.dirty = false; reference: insertBlock,
updateBlock, this.cachedDoc, doc.update, buildBlockNode, blocks.

packages/frontend/src/app/docs/docs-view.tsx (1)

55-57: onEditorReady callback type should explicitly allow null for cleanup.

The cleanup code casts null as unknown as EditorAPI which bypasses type safety. The prop type should explicitly allow null so consumers know to handle both states.

♻️ Suggested fix

 interface DocsViewProps {
-  onEditorReady?: (editor: EditorAPI) => void;
+  onEditorReady?: (editor: EditorAPI | null) => void;
 }

Then the cleanup becomes type-safe:

     return () => {
       editor.dispose();
       editorRef.current = null;
-      onEditorReady?.(null as unknown as EditorAPI);
+      onEditorReady?.(null);
     };

Also applies to: 100-104

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/frontend/src/app/docs/docs-view.tsx` around lines 55 - 57, The prop
onEditorReady in DocsViewProps should allow null to avoid unsafe casting; change
its type to onEditorReady?: (editor: EditorAPI | null) => void and update any
calls (including the cleanup that currently uses null as unknown as EditorAPI)
to pass null directly; search for DocsViewProps and the onEditorReady
invocations (including the cleanup block around the editor teardown) and remove
the null cast so consumers receive EditorAPI | null safely.

docs/tasks/archive/2026/03/20260323-yorkie-doc-store-todo.md (4)

840-846: Consider expanding Design Notes with serialization patterns.

The Design Notes section effectively documents key implementation decisions. Consider adding one more note about boolean attribute serialization:

- **Boolean attribute serialization**: Yorkie Tree attributes are stored as strings. Boolean style properties (bold, italic, etc.) are serialized as the string `'true'` when true, and omitted when false. This requires string comparison during deserialization (e.g., `attrs.bold === 'true'`).

This would complement the existing note about numeric serialization and provide complete documentation of the attribute conversion patterns.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/tasks/archive/2026/03/20260323-yorkie-doc-store-todo.md` around lines
840 - 846, Add a Design Notes bullet documenting boolean attribute
serialization: state that Yorkie Tree node attributes are strings, boolean style
properties (e.g., bold, italic) are serialized as the string 'true' when true
and omitted when false, and deserialization must check string equality (for
example using attrs.bold === 'true'); place this note alongside the existing
numeric serialization note in the Design Notes section to complete attribute
conversion documentation.

---

726-726: Block ID generation may have collision risk.

Using Date.now() for block ID generation (line 726: id: \block-${Date.now()}-0``) can cause collisions if multiple blocks are created within the same millisecond, especially during programmatic initialization or batch operations.

Consider using a more robust ID generation strategy like crypto.randomUUID() or the generateBlockId() utility that's already imported in the tests (line 439).

Alternative using existing utility

+import { generateBlockId } from '@wafflebase/document';
+
 function initialDocsRoot(): YorkieDocsRoot {
   return {
     content: new yorkie.Tree({
       type: 'doc',
       children: [
         {
           type: 'block',
           attributes: {
-            id: `block-${Date.now()}-0`,
+            id: generateBlockId(),
             type: 'paragraph',

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/tasks/archive/2026/03/20260323-yorkie-doc-store-todo.md` at line 726,
Replace the fragile timestamp-based block ID (`id: \`block-${Date.now()}-0\``)
with a robust ID generator; locate the block creation that sets id using
Date.now() and use the existing generateBlockId() utility (already referenced in
tests) or crypto.randomUUID() to eliminate millisecond-collision risk, ensuring
the new ID format matches other consumers of block IDs.

---

354-354: Consider throwing an error for missing block IDs.

Line 354 falls back to an empty string when a block's id attribute is missing: id: attrs.id ?? ''. This could hide data integrity issues. Since every block should have a valid ID, consider throwing an error instead:

-    return { id: attrs.id ?? '', type: 'paragraph', inlines, style };
+    if (!attrs.id) {
+      throw new Error('Block node missing required id attribute');
+    }
+    return { id: attrs.id, type: 'paragraph', inlines, style };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/tasks/archive/2026/03/20260323-yorkie-doc-store-todo.md` at line 354,
The code currently returns a block with a fallback empty id using "id: attrs.id
?? ''", which can hide missing-ID bugs; update the function that constructs this
block to validate attrs.id and throw a descriptive error (e.g., throw new
Error('Missing block id') or use an assert) when attrs.id is null/undefined
instead of defaulting to ''. Replace the fallback expression with a runtime
check before returning the object so every block always has a valid id, and
adjust any callers/tests to expect the thrown error where appropriate.

---

423-627: Consider adding edge case tests.

The test suite provides good coverage of happy paths. Consider adding tests for error conditions and edge cases:

1. Error handling: Test that updateBlock() and deleteBlock() throw errors for non-existent block IDs (mentioned in line 526-528, but could be expanded)
2. Concurrent modifications: Test behavior when tree is modified externally between operations
3. Empty inline handling: Test that empty inline arrays are handled correctly (line 350-352 adds default empty inline)
4. pageSetup defaults: Test getPageSetup() when no pageSetup was ever set (should return defaults from resolvePageSetup)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/tasks/archive/2026/03/20260323-yorkie-doc-store-todo.md` around lines
423 - 627, Add focused unit tests in
packages/frontend/src/app/docs/yorkie-doc-store.test.ts that cover the suggested
edge cases: (1) assert YorkieDocStore.updateBlock and deleteBlock throw for
non-existent IDs (use updateBlock and deleteBlock directly and expect
exceptions), (2) simulate concurrent modifications by calling the underlying
yorkie.Document.update between store operations and verify YorkieDocStore
reflects external tree changes, (3) verify handling of empty inlines by setting
a block with an empty inlines array and ensuring getDocument/getBlock returns an
empty array (and that insert/update preserve empty inlines), and (4) assert
getPageSetup returns the defaults from resolvePageSetup when no pageSetup was
set (call getPageSetup on a fresh YorkieDocStore and compare to
resolvePageSetup()). Ensure tests reference the YorkieDocStore instance and
functions updateBlock, deleteBlock, getPageSetup, and resolvePageSetup so they
run deterministically in-memory.

packages/backend/test/authenticated-http.e2e-spec.ts (1)

145-150: Add a complementary resolve assertion for type: 'doc'.

This verifies the new type field for the default "sheet" path, but it doesn’t protect the "doc" share-link resolve path introduced in this PR. Please add a second case creating a doc-typed document and asserting type: 'doc' in /share-links/:token/resolve.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/backend/test/authenticated-http.e2e-spec.ts` around lines 145 - 150,
Add a second test case that mirrors the existing resolve assertion but creates a
document with type 'doc' and a share-link for that document, then call the same
/share-links/:token/resolve flow and assert the returned body includes type:
'doc' along with documentId, role, and title; locate the existing assertion
using resolveResponse.body and doc in the test and duplicate the
setup/resolve/assert steps for a doc-typed document to validate the "doc"
resolve path.

docs/design/docs-frontend-integration.md (2)

184-196: Consider adding the migration file to the file map.

The file map lists the Prisma schema file but not the migration file. While migrations are auto-generated, explicitly listing it could make the implementation plan clearer.

📋 Suggested addition

Add a row after the schema.prisma entry:

 | File | Action | Description |
 |------|--------|-------------|
 | `packages/backend/prisma/schema.prisma` | Modify | Add `type` field to Document |
+| `packages/backend/prisma/migrations/...` | Create | Generate migration for `type` field |
 | `packages/backend/src/document/document.service.ts` | Modify | Pass `type` to Prisma create |

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/design/docs-frontend-integration.md` around lines 184 - 196, Update the
"File Map" table to include the generated Prisma migration file alongside the
`packages/backend/prisma/schema.prisma` entry: add a row (immediately after the
`schema.prisma` row) that names the migration (e.g.,
`packages/backend/prisma/migrations/<timestamp>_add_document_type/migration.sql`
or the migration folder), Action "Add" or "Create", and Description "Add `type`
field to Document (Prisma migration)"; this makes the plan explicit and ties the
migration to the schema change referenced in the File Map.

---

136-140: Add language specifiers to fenced code blocks.

Two fenced code blocks are missing language specifiers, which can affect rendering and violates markdown best practices.

📝 Proposed fix

For the dropdown menu visual (lines 136-140):

-```
+```text
 [+ New ▾]
   ├── New Sheet
   └── New Document

For the implementation phases (lines 199-213):

```diff
-```
+```text
 Phase 1: Backend (type field)
   1. Prisma schema + migration
   2. Document service + controller updates

Also applies to: 199-213

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/design/docs-frontend-integration.md` around lines 136 - 140, The two
fenced code blocks showing the dropdown menu visual ("[+ New ▾] ├── New Sheet
└── New Document") and the implementation phases ("Phase 1: Backend (type field)
1. Prisma schema + migration 2. Document service + controller updates") are
missing language specifiers; update each triple-backtick fence to include a
language (use "text") so the blocks become ```text ... ``` to ensure correct
rendering and compliance with markdown best practices.

packages/frontend/src/app/docs/yorkie-doc-store.ts (4)

46-57: Potential NaN propagation in parseInlineStyle.

Number(attrs.fontSize) returns NaN if the attribute is an empty string or invalid. Unlike parseBlockStyle which uses normalizeBlockStyle to provide defaults, parseInlineStyle passes through the raw parsed value.

Consider adding a guard:

🛡️ Suggested fix

-  if ('fontSize' in attrs) style.fontSize = Number(attrs.fontSize);
+  if ('fontSize' in attrs) {
+    const parsed = Number(attrs.fontSize);
+    if (!Number.isNaN(parsed)) style.fontSize = parsed;
+  }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/frontend/src/app/docs/yorkie-doc-store.ts` around lines 46 - 57,
parseInlineStyle currently sets fontSize using Number(attrs.fontSize) which can
produce NaN for empty/invalid values; update parseInlineStyle to parse
attrs.fontSize (e.g., with parseFloat or Number) and only assign style.fontSize
when the parsed value is a finite number (use Number.isFinite or !Number.isNaN)
so NaN is not propagated into InlineStyle; keep all other attribute handling
unchanged and ensure fontSize falls back to undefined (or omits the property)
when parsing fails.

---

166-179: Potential NaN values if sub-objects are malformed.

If proxy.paperSize or proxy.margins are undefined or missing properties, Number(undefined) returns NaN. While resolvePageSetup at the call site may normalize these, the function itself could produce invalid values.

Consider adding fallbacks:

🛡️ Suggested fix

 function readPageSetup(proxy: any): PageSetup {
   const ps = proxy.paperSize;
   const m = proxy.margins;
   return {
-    paperSize: { name: ps?.name, width: Number(ps?.width), height: Number(ps?.height) },
+    paperSize: { name: ps?.name, width: Number(ps?.width) || 0, height: Number(ps?.height) || 0 },
     orientation: proxy.orientation ?? 'portrait',
     margins: {
-      top: Number(m?.top),
-      bottom: Number(m?.bottom),
-      left: Number(m?.left),
-      right: Number(m?.right),
+      top: Number(m?.top) || 0,
+      bottom: Number(m?.bottom) || 0,
+      left: Number(m?.left) || 0,
+      right: Number(m?.right) || 0,
     },
   };
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/frontend/src/app/docs/yorkie-doc-store.ts` around lines 166 - 179,
readPageSetup currently calls Number(...) on potentially undefined
sub-properties (proxy.paperSize and proxy.margins) which yields NaN; update
readPageSetup to defensively handle missing/malformed sub-objects by providing
safe fallbacks for paperSize (name defaulting to empty string, width/height
defaulting to 0) and margins (top/bottom/left/right defaulting to 0) before
coercion, e.g., use ps?.width ?? 0 and m?.top ?? 0 patterns when building the
PageSetup return so all numeric fields are always valid numbers; keep the
existing orientation defaulting behavior.

---

292-303: Consider adding bounds validation to insertBlock.

Unlike deleteBlockByIndex which now validates bounds, insertBlock doesn't check that index is within valid range [0, blocks.length]. While Yorkie may handle invalid indices gracefully, explicit validation would provide clearer error messages and consistent behavior with other methods.

🛡️ Suggested fix

 insertBlock(index: number, block: Block): void {
+  const currentDoc = this.getDocument();
+  if (index < 0 || index > currentDoc.blocks.length) {
+    throw new Error(`Block index out of bounds: ${index}`);
+  }
   this.doc.update((root) => {
     const tree = root.content;
     if (!tree || typeof tree.getRootTreeNode !== 'function') return;
     tree.editByPath([index], [index], buildBlockNode(block));
   });
   // Update cache in-place
-  const currentDoc = this.getDocument();
   currentDoc.blocks.splice(index, 0, block);
   this.cachedDoc = currentDoc;
   this.dirty = false;
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/frontend/src/app/docs/yorkie-doc-store.ts` around lines 292 - 303,
insertBlock lacks bounds checking for the index; before mutating the doc call
this.getDocument() to read currentDoc.blocks.length and validate index is
between 0 and currentDoc.blocks.length (inclusive), and if out of range throw a
RangeError with a clear message (consistent with deleteBlockByIndex behavior).
Perform this validation before calling this.doc.update and before splicing the
cachedDoc so you fail fast and keep behavior consistent; leave the existing
calls to buildBlockNode(block), this.doc.update, and the in-place cache splice
unchanged otherwise.

---

190-192: Consider bounding undo/redo stack size.

The undo and redo stacks grow unbounded, which could cause memory pressure during long editing sessions with large documents. Since the comment notes this is "Phase 1," consider adding a maximum stack size.

🛠️ Suggested approach

+  private static readonly MAX_UNDO_STACK_SIZE = 100;
+
   // Local snapshot-based undo/redo (Phase 1)
   private undoStack: Document[] = [];
   private redoStack: Document[] = [];

   snapshot(): void {
     const current = this.getDocument();
     this.undoStack.push(cloneDocument(current));
+    // Limit stack size to prevent unbounded memory growth
+    if (this.undoStack.length > YorkieDocStore.MAX_UNDO_STACK_SIZE) {
+      this.undoStack.shift();
+    }
     this.redoStack = [];
   }

Also applies to: 348-352

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/frontend/src/app/docs/yorkie-doc-store.ts` around lines 190 - 192,
The undoStack and redoStack are unbounded; add a MAX_STACK_SIZE constant (e.g.,
50 or configurable) in the yorkie-doc-store class and enforce it whenever
pushing onto undoStack or redoStack (truncate oldest entries or shift when
length > MAX_STACK_SIZE). Update the push logic used by methods that save
snapshots (referencing undoStack and redoStack) and also apply the same trimming
when moving entries between stacks (the code around the current snapshot
push/restore logic that manipulates undoStack/redoStack). Make MAX_STACK_SIZE
configurable via constructor or a class-level constant and ensure both stacks
are trimmed consistently to prevent unbounded memory growth.