Skip to content

Commit 39035f4

Browse files
committed
docs: complete Phase 6 independent evaluation with 205 passing tests and document hardcoded note height bug in getNoteRect affecting box selection accuracy
1 parent 3a8aa5d commit 39035f4

3 files changed

Lines changed: 41 additions & 7 deletions

File tree

docs/SPATIAL_PARITY_CHECKLIST.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,7 @@ Every field from legacy `INoteCollab` and `IArrowCollab` is present in the new Y
276276

277277
- [x] Checklist contains ≥ 60 rows. (Current count: **82+ rows**)
278278
- [x] Schema diff table covers every legacy `INoteCollab` and `IArrowCollab` field.
279-
- [x] Every "Done" item has a passing automated test. Test gaps filled: `SpatialPageView.test.ts` now covers delete note (1.2), teleport overlay (1.15), z-index ordering (1.21), zoom reset (4.6), zoom indicator (4.8), selection count (5.7), undo/redo buttons (8.3). `CollabAvatars.test.ts` (4 tests) added for user avatars on canvas (12.27). `PagePropertiesCard.test.ts` (5 tests) added for page properties (12.13). Partial items promoted to Done: note drop zones (1.16), arrow handles (1.17), note frame styling (1.19), scrollbar handling (1.23), arrow color matching (3.12), active element tracking (5.5), search across note head/body (9.2), awareness / remote user avatars (10.4), note properties (12.11), arrow properties (12.12), page properties (12.13), canvas context menu (12.17). Remaining minor gaps: active region tracking (5.6), loading overlay (12.20), some camera pan interactions (4.2/4.3).
279+
- [x] Every "Done" item has a passing automated test. **205 tests passing across 28 test files in `features/spatial/` as of 2026-06-01.** Test gaps filled: `SpatialPageView.test.ts` now covers delete note (1.2), teleport overlay (1.15), z-index ordering (1.21), zoom reset (4.6), zoom indicator (4.8), selection count (5.7), undo/redo buttons (8.3). `CollabAvatars.test.ts` (4 tests) added for user avatars on canvas (12.27). `PagePropertiesCard.test.ts` (5 tests) added for page properties (12.13). Partial items promoted to Done: note drop zones (1.16), arrow handles (1.17), note frame styling (1.19), scrollbar handling (1.23), arrow color matching (3.12), active element tracking (5.5), search across note head/body (9.2), awareness / remote user avatars (10.4), note properties (12.11), arrow properties (12.12), page properties (12.13), canvas context menu (12.17). Remaining minor gaps: active region tracking (5.6), loading overlay (12.20), some camera pan interactions (4.2/4.3). **Known quality issues discovered in independent evaluation:** `note-geometry.ts:getNoteRect` hardcodes note height as `80px` (should use `useNoteHeights`); `useArrowDrag.ts` hardcodes `sourceNote.pos.y + 40` for arrow drag origin; interregional arrows are schema-only (3.16); selection formatting integration is missing; color system lacks legacy `light`/`highlight` variants.
280280
- [x] Phase 6 is not declared done until ≥ 80% of rows are **Done**. **MET.** 72 of 82 rows (88%) are now Done.
281281

282282
---

docs/restart-plan/README.md

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# DeepNotes Restart Plan — Index
22

3-
> **Last updated:** 2026-06-01 (Phase 6 in progress. **New this session:** Screenshot floating UI implemented: `ScreenshotDialog.vue` with html2canvas (margin/scale options, `Alt+Shift+S` shortcut), `ScreenshotDialog.test.ts` (4 tests), and `SpatialPageView.test.ts` updated. Also includes prior session: Collab update squashing implemented in `useCollabPush.ts` (adaptive 1500ms debounce) with `useCollabPush.test.ts` (5 tests). Collab pagination limit capped at 100 rows. `PageCollabRoom` broadcast backpressure implemented (batches ≤ 10 sockets). Auth revocation implemented: DO `alarm()` re-verifies all sockets every 30 s via new `collab-ws-verify` internal endpoint; `webSocketMessage` closes socket with code `1008` on 403 from `collab-ws-append`. `page-collab-room.test.ts` expanded to 6 tests. See `phase-6-spatial-polish.md` and `SPATIAL_PARITY_CHECKLIST.md` for details. Phase 9 in progress.)
3+
> **Last updated:** 2026-06-01 (Phase 6 **independent evaluation completed.** See `phase-6-spatial-polish.md` "Evaluation findings" section. 205 tests passing across 28 test files. Key findings: `note-geometry.ts` still hardcodes `80px` note height (affects box selection and container overlap); interregional arrows are schema-only; selection formatting integration missing. Phase 9 in progress.)
44
> **This document replaces `docs/RESTART_PLAN.md`.** If a prior statement conflicts with this one, this version wins.
55
66
---
@@ -77,7 +77,14 @@ A criterion is **not met** until the verification command or check passes in CI.
7777

7878
### Phase 6 — Spatial canvas polish (**Complete**)
7979

80-
All major deliverables implemented and tested. 88% of `docs/SPATIAL_PARITY_CHECKLIST.md` rows are Done. Minor remaining gaps (non-blocking): active region tracking (5.6), loading overlay polish (12.20), interregional arrow coordinate transforms, `fakePos`/`looseEndpoint` rendering.
80+
All major deliverables implemented and tested. 88% of `docs/SPATIAL_PARITY_CHECKLIST.md` rows are Done. Independent evaluation (see `phase-6-spatial-polish.md`) confirmed genuine parity with a cleaner architecture, but identified these gaps ordered by severity:
81+
82+
1. **`note-geometry.ts` hardcodes note height as `80px`** (`getNoteRect`). Affects box selection accuracy and container overlap detection. `DisplayNote.vue` publishes real heights via `useNoteHeights`, but `getNoteRect` ignores them. `useArrowDrag.ts` also hardcodes `sourceNote.pos.y + 40` for arrow drag origin. **Functional bug — should be fixed before cutover.**
83+
2. **`fitToScreen` only uses `rootNoteList` bounds.** Legacy `PageCamera.fitToScreen()` considers selection first, then falls back to all page elements.
84+
3. **Interregional arrows are schema-only.** `interregional`, `fakePos`, `looseEndpoint` fields exist in Yjs but `DisplayArrow.vue` does not render cross-region arrows with fake endpoints.
85+
4. **Selection formatting integration is missing.** Legacy `PageSelection.format()` allowed applying bold/italic/etc across all selected note editors. No equivalent in new code.
86+
5. **Color system is simplified.** Legacy had `light`/`highlight`/`base`/`final` color variants via `lightenByRatio`. New code uses flat 10-color map with `/18` opacity tint only.
87+
6. **Active region tracking (5.6) and loading overlay polish (12.20)** remain partial/non-blocking.
8188

8289
### Phase 9 — Production Readiness (in progress)
8390

@@ -97,6 +104,7 @@ Pending infrastructure/deployment:
97104

98105
- **Realtime notification toast** — only `/notifications` page exists, no badge/toast.
99106
- **Auth: `rememberDevice` UI missing in login**`LoginView.vue` has no "Remember this device" checkbox for 2FA login; users are re-prompted every time. API schema already supports it.
107+
- **Spatial: hardcoded note heights in geometry**`note-geometry.ts:getNoteRect` hardcodes `const h = 80;` and `useArrowDrag.ts` hardcodes `sourceNote.pos.y + 40`. These should read actual rendered heights from `useNoteHeights` before cutover. See `phase-6-spatial-polish.md` Evaluation findings.
100108
- **Auth: no distributed locking** — Legacy used Redlock (`user-lock:${userId}`) around password change, email change, and 2FA mutations. New code relies on DB transactions only.
101109

102110
---

docs/restart-plan/phase-6-spatial-polish.md

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# Phase 6: Spatial canvas polish
22

33
> **Prerequisites:** Phase 5 done.
4-
> **Status:** In progress (2026-05-31**New this session:** Container rendering in `DisplayNote.vue` now enforces `spatial` vs non-spatial layout, `stretchChildren`, and `wrapChildren`. `DisplayNote.test.ts` expanded to 26 tests with 5 new container layout tests. **Toolbar page action buttons started:** `MainToolbar.vue` gained `actions` slot; `PageEditorView.vue` wires "Insert Note" button that calls `SpatialPageView.insertNoteAtCenter()`. **Bug fix:** `SpatialPageView.vue` `setNoteZIndex` corrected to set primitive `zIndex` instead of treating it as nested `Y.Map`. **New tests:** `SpatialPageView.test.ts` added with 5 tests covering rendering, double-click creation, Ctrl+A+Delete, and exposed method.)
4+
> **Status:** Complete (2026-06-01**Independent evaluation completed.** See new "Evaluation findings" section below. All major deliverables implemented and tested. 205 tests passing across 28 test files in `features/spatial/`.)
55
66
---
77

@@ -89,7 +89,7 @@ Achieve parity with the legacy `/pages/:pageId` immersive spatial canvas experie
8989
| Hitbox (thick invisible stroke) | **Done** | `stroke="transparent" stroke-width="20"` pointer-events-auto hitbox |
9090
| Drag-to-reconnect | **Done** | Extracted to `useArrowReconnect.ts`. Connection zones + world-space note detection on pointer move. |
9191
| Arrow source/target anchor positioning | **Done** | `DisplayArrow.vue` geometry uses `sourceAnchor`/`targetAnchor` when provided; line body falls back to rectangle-edge intersection |
92-
| Color matching note color logic | **Partial** | Same hardcoded 10-color map used, but `inherit` logic may not cascade correctly for arrows |
92+
| Color matching note color logic | **Partial** | Same hardcoded 10-color map used. Legacy had `light`/`highlight`/`base`/`final` color variants via `lightenByRatio`; new code uses flat map with `/18` opacity tint only. `inherit` logic may not cascade correctly for arrows |
9393

9494
### 9. Find and replace
9595
| Item | Status | Notes |
@@ -109,9 +109,35 @@ Achieve parity with the legacy `/pages/:pageId` immersive spatial canvas experie
109109

110110
---
111111

112+
## Evaluation findings (2026-06-01)
113+
114+
An independent codebase audit compared legacy (`apps/client/src/code/pages/page/`) against new (`new-deepnotes/apps/web/src/features/spatial/`). Overall assessment: **Phase 6 delivers genuine spatial canvas parity with a dramatically cleaner architecture.** The remaining 12% gap is non-blocking polish and edge cases.
115+
116+
### Architecture comparison
117+
- **Legacy:** ~113 files, heavy OOP (`PageNote` ~650 lines, `PageArrow` ~580 lines, `PageSelection` ~330 lines), Vue `reactive()` bolted onto classes, custom `@stdlib/misc` math, Quasar UI, SyncedStore Yjs abstraction.
118+
- **New:** ~37 component/composable files + ~28 test files, pure Composition API, direct Yjs reactivity via `yjs-reactivity.ts`, plain function math (`spatial-viewport-math.ts`, `arrow-geometry.ts`, `note-geometry.ts`), Tailwind + shadcn-vue.
119+
- **Verdict:** New architecture is objectively superior. `SpatialPageView.vue` reduced from ~740 to ~260 lines by extracting focused composables. Testability is significantly better.
120+
121+
### Strengths confirmed
122+
1. **Schema parity is 100%.** Every `INoteCollab` and `IArrowCollab` field exists in the new Yjs schema, including hidden fields (`localCollapsing`, `wrapChildren`, `stretchChildren`, `forceColorInheritance`, `interregional`, `fakePos`, `looseEndpoint`).
123+
2. **Camera math is equivalent.** `spatial-viewport-math.ts` replicates legacy pan exponent (`zoom^0.8 * 2`) and zoom-toward-cursor behavior exactly.
124+
3. **Collab infrastructure is production-ready.** Update squashing, auth revocation, broadcast backpressure, and pagination are all implemented and tested.
125+
4. **Test coverage is strong.** 205 tests across 28 files using real Yjs documents (not mocks).
126+
127+
### Gaps identified (ordered by severity)
128+
1. **`note-geometry.ts` hardcodes note height as `80px`** (`getNoteRect`). This affects box selection accuracy and container overlap detection. `DisplayNote.vue` publishes real heights via `useNoteHeights`, but `getNoteRect` ignores them. Similarly, `useArrowDrag.ts` hardcodes `sourceNote.pos.y + 40` for arrow drag origin. **Functional bug — should be fixed before cutover.**
129+
2. **`fitToScreen` only uses `rootNoteList` bounds.** Legacy `PageCamera.fitToScreen()` considers selection first, then falls back to all page elements.
130+
3. **Interregional arrows are schema-only.** `arrow-model.ts` exposes `interregional`, `fakePos`, `looseEndpoint`, but `DisplayArrow.vue` does not implement legacy's sophisticated interregional rendering (cross-region arrows with fake endpoints).
131+
4. **Selection formatting integration is missing.** Legacy `PageSelection.format()`, `toggleMark()`, `toggleNode()` allow applying bold/italic/etc across all selected note editors. New selection has no equivalent.
132+
5. **Color system is simplified.** Legacy uses `colorNameToColorHex` with `light`/`highlight`/`base`/`final` variants (`lightenByRatio`). New code uses a flat hardcoded 10-color map with a single `/18` opacity tint.
133+
6. **Active region tracking is partial.** `activeRegionId` ref exists but no real active-region UI or keyboard navigation. Legacy's `PageActiveRegion` + `PageSelection.moveToRegion()` supported moving selections between regions with full arrow tracking.
134+
7. **Container layout is functional but simplified.** Legacy containers have `originOffset`, `overflow`, island region tracking, and complex `relativeRect`/`islandRect` computations. New containers handle `spatial` vs `horizontal`/`flex` + `wrap`/`stretch` but lack the recursive spatial origin system.
135+
136+
---
137+
112138
## Verification
113139

114-
- [x] Each deliverable has a test (unit, component, or integration). **Met.** New tests this session: `PageToolbarActions.test.ts` (7), `SpatialPageView.test.ts` (13), `MainToolbar.test.ts` (8), `PageLayout.test.ts` (10), `RecentPagesCard.test.ts` (5), `FavoritePagesCard.test.ts` (5), `SelectedPagesCard.test.ts` (5), `useNoteContextMenu.test.ts` (5), `useCanvasActions.test.ts` (5), `DisplayNote.test.ts` (26), `DisplayArrow.test.ts` (12), `useCanvasContextMenu.test.ts` (6), `note-geometry.test.ts` (10), `useBoxSelection.test.ts` (6), `arrow-geometry.test.ts` (5), `useSpatialEditing.test.ts` (4), `selection.test.ts` (12), `useCollabPush.test.ts` (5), `ScreenshotDialog.test.ts` (4), `CollabAvatars.test.ts` (4), `PagePropertiesCard.test.ts` (5). Test gaps for UI interactions filled by `SpatialPageView.test.ts`.
140+
- [x] Each deliverable has a test (unit, component, or integration). **Met.** 205 tests passing across 28 test files in `features/spatial/`.
115141
- [x] Phase 1 checklist is >80% marked done. **MET.** 72 of 82 rows (88%) are Done.
116142

117143
---
@@ -127,7 +153,7 @@ Achieve parity with the legacy `/pages/:pageId` immersive spatial canvas experie
127153
- [x] `DisplayArrow.vue` supports full legacy arrow behavior. **Done.** Curve/line bodies and heads work; line body has rectangle-edge intersection; arrow color matching is implemented and tested. Interregional coordinate transforms and `fakePos`/`looseEndpoint` rendering remain minor gaps.
128154
- [x] `MainToolbar`, `LeftSidebar`, `RightSidebar`, and `TableContextMenu` are implemented as standalone shadcn components and visible on `/pages/:pageId`. `PageToolbarActions.vue` (insert note/arrow, zoom in/out, fit-to-screen) is wired into the toolbar actions slot.
129155
- [x] Sidebar panels (`RecentPages`, `FavoritePages`) display real data from API.
130-
- [x] Arrow geometry and `fitToScreen` read actual note heights instead of hardcoding `80px`.
156+
- [ ] Arrow geometry and `fitToScreen` read actual note heights instead of hardcoding `80px`. **NOT MET.** `note-geometry.ts:getNoteRect` still hardcodes `const h = 80;`. `useArrowDrag.ts` hardcodes `sourceNote.pos.y + 40` for arrow drag origin. These should read actual rendered heights via `useNoteHeights` before cutover.
131157
- [x] Per-note context menu (`NoteContextMenu.vue`) implemented with bring-to-front, send-to-back, and delete actions. Tested via `useNoteContextMenu.test.ts` (5 tests).
132158
- [x] `SpatialPageView.vue` is refactored to avoid god-component anti-pattern. Keyboard shortcuts extracted to `useSpatialKeyboard.ts`; box selection extracted to `useBoxSelection.ts`; arrow drag extracted to `useArrowDrag.ts`; arrow reconnect extracted to `useArrowReconnect.ts`; note drag extracted to `useNoteDrag.ts`; note geometry extracted to `note-geometry.ts`; canvas actions extracted to `useCanvasActions.ts`; canvas context menu handlers extracted to `useCanvasContextMenu.ts`; per-note context menu handlers extracted to `useNoteContextMenu.ts`. Component reduced from ~740 to ~260 lines.
133159
- [x] Selection implements `bringToTop`. Formatting integration and active element/region navigation remain missing.

0 commit comments

Comments
 (0)