packages/docs/test/view/incremental-layout.test.ts (1)

65-95: Add one legacy-style edge-case test for missing textIndent/marginLeft.

Given these fields were newly introduced, a regression test for blocks deserialized without them would guard against runtime layout breakage in older docs.

As per coding guidelines, "Write tests for critical business logic and edge cases".

🤖 Prompt for AI Agents

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

In `@packages/docs/test/view/incremental-layout.test.ts` around lines 65 - 95, Add
a legacy-style test that ensures deserialized blocks missing style.textIndent
and style.marginLeft don't break layout and default to 0: create a block via
makeBlock('...') but explicitly delete or leave style.textIndent and
style.marginLeft undefined, call computeLayout([block], mockCtx(), width) and
assert the layout completes (no exception) and that lines[0].runs[0].x and
lines[1].runs[0].x are 0 (or >=0 if only checking non-negative), referencing
makeBlock, computeLayout, mockCtx and result.layout.blocks[0].lines to locate
the code to change.

packages/docs/src/view/ruler.ts (3)

15-22: Simplify detectUnit locale matching logic.

The current condition locale.startsWith(l.split('-')[0]) && locale === l is redundant — if locale === l, then locale.startsWith(...) is always true. This can be simplified to INCH_LOCALES.includes(locale).

♻️ Suggested simplification

 export function detectUnit(locale: string | undefined): RulerUnit {
   if (!locale) return 'inch';
-  if (INCH_LOCALES.some((l) => locale.startsWith(l.split('-')[0]) && locale === l)) {
+  if (INCH_LOCALES.includes(locale)) {
     return 'inch';
   }
   if (locale.startsWith('en')) return 'inch';
   return 'cm';
 }

🤖 Prompt for AI Agents

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

In `@packages/docs/src/view/ruler.ts` around lines 15 - 22, The detectUnit
function uses a redundant check when matching INCH_LOCALES; replace the
condition inside the INCH_LOCALES.some callback with a direct membership check
(use INCH_LOCALES.includes(locale)) so that detectUnit (and its branch that
currently reads INCH_LOCALES.some((l) => locale.startsWith(l.split('-')[0]) &&
locale === l)) simply tests includes(locale) to simplify and clarify the logic.

---

211-219: Label calculation for cm unit is overly complex and potentially incorrect.

The calculation at lines 212-217 is convoluted:

• labelValue uses a complex formula that may not produce correct cm values
• labelText uses a different formula than labelValue

For cm, since subdivisions = 10 and each subdivision represents 1mm, the label at major ticks should simply be i / subdivisions (representing centimeters), which is the same logic as inches.

♻️ Suggested simplification

       // Draw label at major ticks
       if (i % subdivisions === 0 && i > 0) {
-        const labelValue = this.unit === 'inch'
-          ? i / subdivisions
-          : Math.round(i * minorStepPx / (this.grid.majorStepPx / subdivisions) / subdivisions * 10) / 10;
-        const labelText = this.unit === 'inch'
-          ? String(i / subdivisions)
-          : String(Math.round(i * minorStepPx * subdivisions / majorStepPx));
+        const labelText = String(i / subdivisions);
         ctx.fillText(labelText, x, 1);
       }

🤖 Prompt for AI Agents

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

In `@packages/docs/src/view/ruler.ts` around lines 211 - 219, The cm label math in
view/ruler.ts is overcomplicated and inconsistent between labelValue and
labelText; update the branch that computes labels when this.unit === 'cm' so
both labelValue and labelText use the simple centimeters formula i /
subdivisions (same as the inch branch) instead of the current expressions
involving minorStepPx/majorStepPx, and use that labelValue (formatted as needed)
when calling ctx.fillText so major tick labels for cm match the intended
centimeter values; adjust any formatting/rounding consistently where labelValue
is converted to string.

---

103-104: Canvas context retrieval may return null.

getContext('2d') can return null if the canvas context is unavailable. The current code casts to CanvasRenderingContext2D without null checking. While unlikely in browser environments, this could cause issues.

🛡️ Defensive check

-    this.hCtx = (this.hCanvas.getContext?.('2d') ?? {}) as CanvasRenderingContext2D;
-    this.vCtx = (this.vCanvas.getContext?.('2d') ?? {}) as CanvasRenderingContext2D;
+    const hCtx = this.hCanvas.getContext?.('2d');
+    const vCtx = this.vCanvas.getContext?.('2d');
+    if (!hCtx || !vCtx) {
+      throw new Error('Failed to get 2D canvas context for ruler');
+    }
+    this.hCtx = hCtx;
+    this.vCtx = vCtx;

🤖 Prompt for AI Agents

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

In `@packages/docs/src/view/ruler.ts` around lines 103 - 104, The current
assignments to hCtx and vCtx cast getContext('2d') results to
CanvasRenderingContext2D without null checks; change the code to call
hCanvas.getContext('2d') and vCanvas.getContext('2d'), check each result for
null, and either throw a clear error (e.g., "Failed to get 2D context for
horizontal/vertical ruler") or provide a safe fallback before assigning to
this.hCtx and this.vCtx so the rest of the Ruler class methods (hCtx, vCtx)
never operate on null.

docs/tasks/active/20260322-docs-ruler-todo.md (1)

1-13: Fix heading hierarchy: use ## before ###.

The document jumps from # (h1) at line 6 to ### (h3) at line 13, skipping h2. This violates markdown heading hierarchy best practices and was flagged by markdownlint.

📝 Suggested fix for heading structure

 ---
 
+## Tasks
+
 ### Task 1: Add `textIndent` and `marginLeft` to BlockStyle

🤖 Prompt for AI Agents

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

In `@docs/tasks/active/20260322-docs-ruler-todo.md` around lines 1 - 13, The
markdown heading hierarchy skips from a top-level `# Document Ruler
Implementation Plan` to `### Task 1: Add 'textIndent' and 'marginLeft' to
BlockStyle`; update that `###` to `##` so the second-level heading is correctly
nested under the main title, ensuring proper heading order and fixing the
markdownlint violation.

docs/design/docs-ruler.md (1)

37-45: Add language identifier to fenced code block.

The DOM structure code block is missing a language identifier. Since this is pseudocode/HTML-like syntax, consider using text or html for clarity.

📝 Suggested fix

-```
+```html
 <container style="overflow: auto">
   <ruler-corner  style="position:sticky; top:0; left:0; z-index:3; 20×20px">

🤖 Prompt for AI Agents

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

In `@docs/design/docs-ruler.md` around lines 37 - 45, The fenced code block
showing the DOM structure lacks a language marker; update the opening
triple-backticks to include a language (e.g., ```html or ```text) so the snippet
for elements like <container>, <ruler-corner>, <h-ruler-canvas>,
<v-ruler-canvas>, <doc-canvas>, and <spacer> is properly identified and
rendered; change only the opening fence to ```html (or ```text) without altering
the snippet content.

packages/docs/test/view/ruler.test.ts (1)

34-40: Incorrect test assertion: snapToGrid(7, 12) should return 0, not 12.

The snapToGrid function uses Math.round(px / step) * step. For snapToGrid(7, 12):

• 7 / 12 ≈ 0.583
• Math.round(0.583) = 1
• 1 * 12 = 12

Wait, actually this is correct since Math.round(0.583) = 1. However, the comment says "snaps to nearest minor step" — this test may be confusing because 7 is closer to 12 than to 0, but only marginally. Consider adding a clearer edge case like snapToGrid(5, 12) which should return 0 (since 5/12 ≈ 0.417 rounds to 0).

💡 Suggested additional edge case for clarity

   it('snapToGrid snaps to nearest minor step for inch', () => {
     const grid = getGridConfig('inch');
     expect(snapToGrid(13, grid.minorStepPx)).toBe(12);
     expect(snapToGrid(7, grid.minorStepPx)).toBe(12);
+    expect(snapToGrid(5, grid.minorStepPx)).toBe(0); // 5 is closer to 0 than 12
     expect(snapToGrid(0, grid.minorStepPx)).toBe(0);
     expect(snapToGrid(96, grid.minorStepPx)).toBe(96);
   });

🤖 Prompt for AI Agents

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

In `@packages/docs/test/view/ruler.test.ts` around lines 34 - 40, The test
assertion is confusing for the "nearest minor step" behavior: replace the
ambiguous case expect(snapToGrid(7, grid.minorStepPx)).toBe(12) with a clearer
edge case that demonstrates rounding down (e.g., expect(snapToGrid(5,
grid.minorStepPx)).toBe(0)), keeping the other assertions intact; locate and
update the assertion in the it block that calls snapToGrid in ruler.test.ts
(reference function snapToGrid and variable grid.minorStepPx).