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

165-170: Avoid calling getBlockType() twice.

editor.getBlockType() is invoked twice in adjacent lines. Store the result in a variable to avoid redundant computation.

♻️ Proposed fix

                 <span className="truncate">
-                  {editor ? getBlockLabel(
-                    editor.getBlockType().type,
-                    editor.getBlockType().headingLevel,
-                  ) : "Normal text"}
+                  {editor ? (() => {
+                    const blockType = editor.getBlockType();
+                    return getBlockLabel(blockType.type, blockType.headingLevel);
+                  })() : "Normal text"}
                 </span>

Alternatively, compute this outside the JSX for better readability:

const currentBlockType = editor?.getBlockType();
const blockLabel = currentBlockType 
  ? getBlockLabel(currentBlockType.type, currentBlockType.headingLevel)
  : "Normal text";

🤖 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 165
- 170, The JSX calls editor.getBlockType() twice; cache its result to avoid
redundant calls by creating a local variable (e.g., currentBlockType) before the
return and use that when computing the label via
getBlockLabel(currentBlockType.type, currentBlockType.headingLevel) or falling
back to "Normal text"; update the span to use the cached
blockLabel/currentBlockType instead of calling editor.getBlockType() again so
only one getBlockType() invocation occurs.

packages/docs/src/view/text-editor.ts (1)

724-739: Extract max list level to a named constant.

The maximum list level of 8 appears as a magic number. Consider defining it as a constant (possibly in model/types.ts alongside LIST_INDENT_PX) for consistency with editor.ts which also needs this bound.

♻️ Proposed improvement

+const MAX_LIST_LEVEL = 8;
+
 private handleTab(shift: boolean): void {
   const block = this.doc.getBlock(this.cursor.position.blockId);
   if (block.type !== 'list-item') return;

   this.saveSnapshot();
   const currentLevel = block.listLevel ?? 0;
-  const newLevel = shift ? Math.max(0, currentLevel - 1) : Math.min(8, currentLevel + 1);
+  const newLevel = shift ? Math.max(0, currentLevel - 1) : Math.min(MAX_LIST_LEVEL, currentLevel + 1);
   if (newLevel === currentLevel) return;

🤖 Prompt for AI Agents

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

In `@packages/docs/src/view/text-editor.ts` around lines 724 - 739, Replace the
magic number 8 in handleTab with a named constant: define e.g. LIST_MAX_LEVEL
(or MAX_LIST_LEVEL) next to LIST_INDENT_PX in model/types.ts, export it, and
import it into packages/docs/src/view/text-editor.ts; then use LIST_MAX_LEVEL in
the Math.min call when computing newLevel and update any other places (like
editor.ts) that currently use 8 to reference the same exported constant so the
bound is consistent across the codebase.

packages/docs/src/view/doc-canvas.ts (3)

272-286: Remove unused lineX parameter.

The lineX parameter is passed to renderListMarker but never used in the function body. Either remove it or document why it's reserved for future use.

♻️ Proposed fix

   private renderListMarker(
     block: Block,
-    lineX: number,
     lineY: number,
     lineHeight: number,
     markerX: number,
     markerText: string,
   ): void {

And update the call site at line 164:

-            this.renderListMarker(block, pageX + pl.x, pageY + pl.y, pl.line.height, markerX, marker);
+            this.renderListMarker(block, pageY + pl.y, pl.line.height, markerX, marker);

🤖 Prompt for AI Agents

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

In `@packages/docs/src/view/doc-canvas.ts` around lines 272 - 286, The
renderListMarker function signature includes an unused parameter lineX; remove
lineX from the parameter list in renderListMarker and update every call site
that passes lineX to instead pass only the remaining parameters (ensure calls to
renderListMarker now match the new signature: (block, lineY, lineHeight,
markerX, markerText)); after changes, run the TypeScript build to catch any
missed call sites and adjust any affected variable names or argument ordering
accordingly.

---

160-160: Document the magic number in marker positioning.

The -4 offset in the marker X calculation lacks explanation. Consider extracting it to a named constant or adding a comment explaining its purpose (e.g., visual centering adjustment).

📝 Suggested improvement

+            const MARKER_OFFSET_ADJUSTMENT = 4; // Fine-tune horizontal centering of marker glyph
-            const markerX = pageX + margins.left + LIST_INDENT_PX * level + LIST_INDENT_PX / 2 - 4;
+            const markerX = pageX + margins.left + LIST_INDENT_PX * level + LIST_INDENT_PX / 2 - MARKER_OFFSET_ADJUSTMENT;

🤖 Prompt for AI Agents

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

In `@packages/docs/src/view/doc-canvas.ts` at line 160, The marker X position uses
a unexplained magic offset (-4) in the calculation for markerX; replace this
literal with a clearly named constant (e.g., MARKER_CENTER_OFFSET_PX or
LIST_MARKER_VISUAL_ADJUST_PX) or add an inline comment explaining it is a visual
centering adjustment, and update the expression where markerX is computed so the
constant name (or comment) documents purpose; ensure the new constant is
colocated near related layout constants (like LIST_INDENT_PX) for
discoverability.

---

68-68: Consider caching list counters during layout computation.

computeListCounters is called on every render frame. For documents with many list items, this may introduce unnecessary overhead. Since list counters only change when document structure changes, consider computing them once during computeLayout and including them in DocumentLayout.

🤖 Prompt for AI Agents

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

In `@packages/docs/src/view/doc-canvas.ts` at line 68, computeListCounters is
being recomputed on every render (call at doc-canvas: listCounters =
computeListCounters(layout.blocks.map(b => b.block))), causing unnecessary work;
move its computation into computeLayout and store the result on the returned
DocumentLayout so renders reuse it. Update computeLayout (and the DocumentLayout
type) to produce a listCounters Map<string,string> alongside existing layout
data, remove the computeListCounters call from render (doc-canvas) and read
layout.listCounters instead, and ensure any code paths that mutate document
structure trigger a recompute via computeLayout so the cached counters stay
correct.

packages/docs/src/view/layout.ts (1)

161-163: Consider extracting HR_HEIGHT as a module-level constant.

The horizontal rule height is defined inline. If this value is needed elsewhere (e.g., for cursor positioning or rendering), extracting it would improve maintainability.

♻️ Proposed improvement

+/** Fixed height for horizontal-rule blocks in pixels. */
+const HR_HEIGHT = 20;
+
 // In computeLayout:
     if (block.type === 'horizontal-rule') {
-      const HR_HEIGHT = 20;
       lines = [{ runs: [], y: 0, height: HR_HEIGHT, width: availableWidth }];

🤖 Prompt for AI Agents

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

In `@packages/docs/src/view/layout.ts` around lines 161 - 163, Extract the inline
HR_HEIGHT constant used in the horizontal-rule branch into a module-level (and
exported if used elsewhere) constant like HORIZONTAL_RULE_HEIGHT so other code
(cursor positioning or render logic) can reuse it; update the branch where
block.type === 'horizontal-rule' to reference the new module-level constant
(instead of the local HR_HEIGHT) and adjust any other places that need the same
value to import/use that constant to keep a single source of truth.

packages/docs/src/view/editor.ts (1)

576-580: Consider sharing INDENT_STEP constant with LIST_INDENT_PX.

INDENT_STEP = 36 is defined locally and matches LIST_INDENT_PX from model/types.ts. Using the shared constant would ensure they stay synchronized.

♻️ Proposed fix

Add to imports:

-import type { InlineStyle, BlockStyle, BlockType, HeadingLevel } from '../model/types.js';
+import type { InlineStyle, BlockStyle, BlockType, HeadingLevel } from '../model/types.js';
+import { LIST_INDENT_PX } from '../model/types.js';

Then use it:

       } else {
-        const INDENT_STEP = 36;
         doc.applyBlockStyle(block.id, {
-          marginLeft: (block.style.marginLeft ?? 0) + INDENT_STEP,
+          marginLeft: (block.style.marginLeft ?? 0) + LIST_INDENT_PX,
         });
       }

🤖 Prompt for AI Agents

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

In `@packages/docs/src/view/editor.ts` around lines 576 - 580, Replace the local
INDENT_STEP = 36 with the shared constant LIST_INDENT_PX from the model types so
the indent step stays synchronized; import LIST_INDENT_PX and use it in the
doc.applyBlockStyle call (referencing block.id and block.style.marginLeft)
instead of defining INDENT_STEP locally.

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

165-170: Minor optimization: avoid duplicate getBlockType() call.

editor.getBlockType() is called twice. Consider destructuring once for slightly cleaner code:

♻️ Proposed refactor

-                  {editor ? getBlockLabel(
-                    editor.getBlockType().type,
-                    editor.getBlockType().headingLevel,
-                  ) : "Normal text"}
+                  {editor
+                    ? (() => {
+                        const { type, headingLevel } = editor.getBlockType();
+                        return getBlockLabel(type, headingLevel);
+                      })()
+                    : "Normal text"}

Or extract to a local variable before the JSX.

🤖 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 165
- 170, The JSX calls editor.getBlockType() twice; to avoid the duplicate call,
read and destructure the block type once into a local variable (e.g., const
blockType = editor.getBlockType()) before the return/JSX and then pass
blockType.type and blockType.headingLevel into getBlockLabel; update the span to
use that local variable and keep the fallback "Normal text" when editor is
falsy.

packages/docs/src/view/text-editor.ts (1)

780-796: Inconsistent max indent level between Tab and Cmd+].

handleTab caps the list level at 8 (line 754: Math.min(8, currentLevel + 1)), but handleIndent has no upper bound (line 787: (block.listLevel ?? 0) + 1). This allows unbounded nesting via Cmd+] while Tab is constrained.

Consider applying the same cap for consistency:

♻️ Proposed fix to add consistent max level

   private handleIndent(): void {
     const INDENT_STEP = 36;
+    const MAX_LIST_LEVEL = 8;
     const block = this.doc.getBlock(this.cursor.position.blockId);
     this.saveSnapshot();
     if (block.type === 'list-item') {
+      const newLevel = Math.min(MAX_LIST_LEVEL, (block.listLevel ?? 0) + 1);
       this.doc.setBlockType(block.id, 'list-item', {
         listKind: block.listKind,
-        listLevel: (block.listLevel ?? 0) + 1,
+        listLevel: newLevel,
       });
     } else {

🤖 Prompt for AI Agents

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

In `@packages/docs/src/view/text-editor.ts` around lines 780 - 796, handleIndent
currently increases list nesting without an upper bound causing inconsistency
with handleTab; update handleIndent to cap the list level the same way (use
Math.min(8, (block.listLevel ?? 0) + 1)) when calling this.doc.setBlockType for
list-item blocks, and ensure non-list indentation remains unchanged (INDENT_STEP
logic for applyBlockStyle stays as-is); locate handleIndent, INDENT_STEP,
getBlock, saveSnapshot, setBlockType, and applyBlockStyle to make the change.