Claude/console dimensionality - #93
Conversation
Access flat.response only on the failed HarnessMcpResult branch so next build can complete. Co-authored-by: Cursor <cursoragent@cursor.com>
…cade Coerce param bindings to string maps and loosen createAtomStoreFacade actions typing so next build can finish. Co-authored-by: Cursor <cursoragent@cursor.com>
Collapse island vocabulary to block placements and free geometry, host rail/dock/ground regions without chrome strips, and add header drag, container collision, and body section IA per the handoff. Co-authored-by: Cursor <cursoragent@cursor.com>
Drop onto a kanban column moves the view-instance under the board and stamps kanbanColumn in config, with layout persist and config merge. Co-authored-by: Cursor <cursoragent@cursor.com>
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR replaces the console’s island-oriented layout model with block placements, geometry, drag-and-drop, nesting, shell markup, registry contracts, and related validation. It also updates harness handling, view descriptors, end-to-end selectors, and implementation documentation. ChangesConsole block model
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Sidebar
participant BlockArrangementHost
participant BlockCanvas
participant ConsoleBlockHost
Sidebar->>BlockArrangementHost: drag landmark into ground arrangement
BlockArrangementHost->>BlockCanvas: render block and placement zones
BlockCanvas->>BlockArrangementHost: report promotion or reorder
BlockArrangementHost->>ConsoleBlockHost: emit placement actions
ConsoleBlockHost-->>BlockArrangementHost: return applied move receipts
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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 |
There was a problem hiding this comment.
Pull request overview
This PR advances the Console “one-block model” migration by replacing the legacy island/mount grammar with a unified block placement + free-geometry canvas model, introducing container-capable blocks (Kanban), and updating shell/UI structure, tests, and gates to match.
Changes:
- Replace
MountPoint/island semantics withBlockPlacement,defaultSize,BlockGeometry, and container support (acceptsChildren) in@commonplace/block-view. - Swap the grid-based island arrangement for a free-geometry
BlockCanvas+BlockArrangementHost, updating shell wiring, move receipts, and Playwright coverage. - Update Console views and substrate layout sections (Card/Workspace), plus add a Kanban container block implementation.
Reviewed changes
Copilot reviewed 52 out of 53 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Lockfile updates for ESLint resolver/import dependency graph changes. |
| packages/block-view/src/types.ts | Introduces one-block model types (placement/geometry/limits/children) and render-props instance. |
| packages/block-view/src/registry.ts | Registry API rename blocksForMount → blocksForPlacement. |
| packages/block-view/src/registry.test.ts | Updates tests for placement filtering + container descriptor shape. |
| packages/block-view/src/island-class.ts | Removed (replaced by block-class). |
| packages/block-view/src/island-class.test.ts | Removed (replaced by block-class tests). |
| packages/block-view/src/index.ts | Re-exports block-class instead of island-class. |
| packages/block-view/src/block-class.ts | New: surface-class defaulting + homogeneous-block defect helper. |
| packages/block-view/src/block-class.test.ts | New: tests for block-class helpers. |
| packages/block-view/package.json | Export map rename ./island-class → ./block-class. |
| docs/plans/console/one-block-model/README.md | New documentation entry point for the one-block model plan. |
| docs/plans/console/one-block-model/implementation-plan.md | New: detailed one-block model implementation plan and acceptance proofs. |
| apps/console/src/views/workspace/WorkspaceSubstrateView.tsx | Adds data-block-section structure and layout adjustments for substrate UI. |
| apps/console/src/views/registry.tsx | Migrates descriptors to placements/defaultSize and registers mail views. |
| apps/console/src/views/CardView.tsx | Adds data-block-section wrappers for identity/substance/relations sections. |
| apps/console/src/views/blocks/KanbanBlock.tsx | New: container-capable Kanban block with droppable columns and nested children rendering. |
| apps/console/src/views/blocks/DeclaredBlocks.tsx | Uses BlockEmptyBody; replaces placeholder Kanban with real KanbanBlock export. |
| apps/console/src/views/blocks/BlockEmptyBody.tsx | Renames component + data attributes from island → block empty state. |
| apps/console/src/views/blocks/AutomationHistoryView.tsx | Uses BlockEmptyBody instead of IslandEmptyBody. |
| apps/console/src/lib/state/store-facade.ts | Loosens facade actions typing to Partial<TState>. |
| apps/console/src/lib/state/proactivity-state.ts | Type tweak for Set<string> initialization. |
| apps/console/src/lib/island-promotion.ts | Removed (replaced by block-placement helpers). |
| apps/console/src/lib/island-promotion.test.ts | Removed (replaced by block-placement tests). |
| apps/console/src/lib/island-grid.ts | Removed (replaced by block-geometry and BlockCanvas). |
| apps/console/src/lib/island-grid.test.ts | Removed (replaced by block-geometry tests). |
| apps/console/src/lib/console-host.ts | Merges config patches instead of overwriting, improving partial config updates. |
| apps/console/src/lib/console-host.test.ts | Adds coverage for nesting + config merge behavior (kanbanColumn). |
| apps/console/src/lib/block-placement.ts | New: reorder/place/nest helpers; size/geometry readers; kanban column helpers. |
| apps/console/src/lib/block-placement.test.ts | New: unit tests for block-placement helpers. |
| apps/console/src/lib/block-move-receipts.ts | Renames island move receipt attribute/functions to block equivalents. |
| apps/console/src/lib/block-geometry.ts | New: free-geometry helpers, clamps, and header-fit guards. |
| apps/console/src/lib/block-geometry.test.ts | New: tests for geometry clamp/limits/header-fit behavior. |
| apps/console/src/lib/block-collision.ts | New: dnd-kit collision rule favoring innermost accepting container. |
| apps/console/src/lib/block-collision.test.ts | New: basic export test for collision factory. |
| apps/console/src/components/shell/ViewInstanceHost.tsx | Updates to BlockShell usage, placement semantics, and query-less container rendering. |
| apps/console/src/components/shell/StatusBar.tsx | Removed (connection UI moved into rail/sidebar). |
| apps/console/src/components/shell/Sidebar.tsx | Adds rail region semantics, run + reconnect widgets, and placement promotion updates. |
| apps/console/src/components/shell/MainToolbar.tsx | Removed (run widget/surface nav consolidated elsewhere). |
| apps/console/src/components/shell/IntuiShell.tsx | Switches IslandArrangementHost → BlockArrangementHost; restructures regions (rail/dock/ground). |
| apps/console/src/components/blocks/kind-glyph.tsx | Renames glyph type to BlockKindGlyph. |
| apps/console/src/components/blocks/IslandGrid.tsx | Removed (replaced by BlockCanvas). |
| apps/console/src/components/blocks/IslandArrangementHost.tsx | Removed (replaced by BlockArrangementHost). |
| apps/console/src/components/blocks/BlockShell.tsx | Updates shell semantics to blocks; supports whole-header drag wiring; uses block-geometry header-fit. |
| apps/console/src/components/blocks/BlockShell.test.tsx | Updates tests for BlockShell and new data attributes. |
| apps/console/src/components/blocks/BlockCanvas.tsx | New: free-geometry canvas with resize handles and container-aware drop behavior. |
| apps/console/src/components/blocks/BlockArrangementHost.tsx | New: host adapter wiring reorder/geometry/placement/nesting emits with move receipts. |
| apps/console/src/app/api/harness/plan/route.ts | Enforces string-only bindings for plan parameter bindings. |
| apps/console/src/app/api/harness/boot/route.ts | Refines 401/403 passthrough and consolidates degraded “missing” reason composition. |
| apps/console/scripts/check-block-classes.mjs | Renames/updates class gate from islands → blocks and keeps luminance delta enforcement. |
| apps/console/package.json | Renames gate script gate:islands → gate:blocks and updates gates aggregator. |
| apps/console/e2e/signatures.spec.ts | Updates selectors and language for BlockShell signatures. |
| apps/console/e2e/island-stripe-promotion.spec.ts | Renames to ground↔rail placement flow and updates selectors/receipt attr. |
| apps/console/e2e/console-sidebar.spec.ts | Updates selectors for new block arrangement host and receipts attribute. |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <div className="grid min-h-0 flex-1 grid-cols-3"> | ||
| <section className="flex min-h-0 flex-col border-r border-ij-seam bg-ij-chrome" aria-label="Project entity tree"> | ||
| <div className="col-span-2 grid min-h-0 grid-cols-2 border-r border-ij-seam"> | ||
| <div className="flex min-h-0 flex-col border-r border-ij-seam" aria-label="Project entity tree"> |
| </div> | ||
|
|
||
| <section className="min-h-0 overflow-auto border-r border-ij-seam p-3" aria-label="Workspace entity details"> | ||
| <div className="min-h-0 overflow-auto border-r border-ij-seam p-3" aria-label="Workspace entity details"> |
|
|
||
| <section className="flex min-h-0 flex-col" aria-label="Local history"> | ||
| <div className="border-b border-ij-seam bg-ij-chrome px-3 py-2"> | ||
| <div data-block-section="revise" className="flex min-h-0 flex-col" aria-label="Local history"> |
| /** The view-instance being rendered. Required for container parenting. */ | ||
| readonly instance?: ObjectRef; |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c39105e15c
ℹ️ 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".
| const fallback = descriptor.block?.defaultSize ?? 'm'; | ||
| const size = readBlockSize(instance, fallback); | ||
| const geometry = readBlockGeometry(instance, fallback); |
There was a problem hiding this comment.
On a fresh /cards layout, both seeded instances have only config.size and no geometry, so this fallback derives { col: 1, row: 1 } for each. The canvas therefore places cards.vi-records directly over cards.vi-grid; reordering only changes sibling order and cannot separate them. Assign initial positions when geometry is absent, or migrate the seeded layout to persist distinct geometries.
AGENTS.md reference: apps/console/AGENTS.md:L133-L135
Useful? React with 👍 / 👎.
| if (overData?.type === 'promote' && overData.zone) { | ||
| onPromote?.(activeBlockId, overData.zone); | ||
| return; |
There was a problem hiding this comment.
This accepts every promotion zone without checking the active item's declared block.placements. For example, the current Cards ground contains record.table, which declares only ground/full/rail, yet the canvas supplies dock zones for all companion regions, so dropping it on "Dock as tool" moves it into an unsupported dock. The tool window force-renders that instance and offers no return-to-ground action outside the rail tray, leaving the block effectively removed from the canvas. Filter zones per descriptor or reject unsupported drops.
Useful? React with 👍 / 👎.
| const onMove = (moveEvent: PointerEvent) => { | ||
| const dCol = Math.round((moveEvent.clientX - startX) / colW); | ||
| const dRow = Math.round((moveEvent.clientY - startY) / rowH); | ||
| if (dCol === 0 && dRow === 0) return; | ||
| const next = clampGeometry(applyEdgeDelta(start, edge, dCol, dRow), limits); | ||
| onGeometryChange(item.viewInstanceId, next); |
There was a problem hiding this comment.
A resize emits an update on every pointer move, and each call is deliberately unawaited. In production, ConsoleBlockHost.emit starts a separate write-through HTTP update for each one, with no per-instance queue or debounce, so a slower earlier request can arrive last and overwrite the final geometry on the server. The local cache looks correct until reload, when the block snaps back to that stale persisted geometry. Debounce the gesture or serialize the writes and flush the final geometry on pointer-up.
AGENTS.md reference: apps/console/AGENTS.md:L133-L135
Useful? React with 👍 / 👎.
| <Sidebar | ||
| host={host} | ||
| surfaces={primarySurfaces} | ||
| companions={companions} |
There was a problem hiding this comment.
Removing MainToolbar removes the only account trigger, while the replacement sidebar receives primarySurfaces, which contains only the five route-backed surfaces. console-account has no route and no remaining host.activateSurface call, so users cannot open Account or sign in after this change; the existing e2e/account.spec.ts still tries to click the now-missing [data-account-trigger]. Add an account affordance to the rail or retain another reachable route.
Useful? React with 👍 / 👎.
| </button> | ||
| <span className="flex size-ij-control shrink-0 items-center justify-center rounded-full bg-ij-raised text-sm text-ij-ink">{initials}</span> | ||
| <span className="min-w-0 truncate text-sm" style={{ opacity: collapsed ? 0 : 1, transition: 'opacity var(--ij-motion) var(--ij-ease)' }}>{tenant}</span> | ||
| {needsReconnect ? ( |
There was a problem hiding this comment.
The rail renders its only connection UI only while needsReconnect is true. Pressing Reconnect immediately sets the state to connecting, which makes that condition false and removes the indicator while the probe is pending. The deleted status bar was also the sole consumer of progressLabel, so changefeed states such as "Connecting live feed" and "Live feed stale" are now invisible. Keep a rail status indicator for connecting and progress states.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
apps/console/src/components/blocks/BlockShell.tsx (1)
73-99: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
ViewInstanceHost.tsxshould not passdraggablethrough on the shell path.BlockShellonly mountsuseDraggablewhendraggableis true and noheaderDragListenersare present, but this host renders the shell outsideBlockCanvas'sDndContextfor tool windows and editor tabs. Setdraggable={false}here, and letBlockCanvasown drag activation.🤖 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 `@apps/console/src/components/blocks/BlockShell.tsx` around lines 73 - 99, Update the ViewInstanceHost shell rendering to explicitly pass draggable={false}. Keep drag activation owned by BlockCanvas and prevent BlockShell’s useDraggable path from mounting when rendered outside its DndContext.
🧹 Nitpick comments (1)
apps/console/src/lib/block-collision.test.ts (1)
4-7: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftTest collision selection behavior.
Line 6 only validates the return type. Add cases for accepted-container priority, rejected descriptor filtering, nesting depth, and geometric fallback so regressions in drag target selection are caught.
🤖 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 `@apps/console/src/lib/block-collision.test.ts` around lines 4 - 7, Add behavioral tests around createBlockCollisionDetection, covering accepted-container priority, filtering rejected descriptors, selecting the correct nesting depth, and geometric fallback. Keep the existing factory return-type assertion, and verify each case’s selected collision target rather than only checking that the factory returns a function.
🤖 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 `@apps/console/package.json`:
- Around line 21-25: Update the “gates” script in package.json to include the
existing “test:e2e” command, ensuring npm run gates executes end-to-end tests in
addition to the current gate checks.
In `@apps/console/src/components/blocks/BlockCanvas.tsx`:
- Around line 189-214: Update onResizeStart so pointermove only tracks the
latest clamped geometry and does not call onGeometryChange for intermediate
steps; commit the final geometry once from the pointerup handler. Ensure
pointermove and pointerup listeners are removed both after pointerup and when
the component unmounts, using the component’s existing effect/lifecycle cleanup
mechanism and preserving the current resize calculations.
In `@apps/console/src/lib/block-placement.ts`:
- Around line 46-62: Update resizeBlockAction and setBlockGeometryAction to
merge the existing view-instance config before applying size or geometry
changes, preserving sibling keys such as kanbanColumn. Reuse the established
current-config lookup and ensure updateViewInstanceConfigAction receives the
merged configuration rather than a partial object.
In `@apps/console/src/lib/console-host.ts`:
- Around line 761-763: Serialize or coalesce the write-through operations
initiated by the action-handling path around writeThroughLayoutUpdates so each
view instance persists updates in order. Ensure newer resize or nesting patches
cannot be overwritten by stale config snapshots when pointer-driven updates
complete out of order, and add coverage that exercises reversed completion
order.
---
Outside diff comments:
In `@apps/console/src/components/blocks/BlockShell.tsx`:
- Around line 73-99: Update the ViewInstanceHost shell rendering to explicitly
pass draggable={false}. Keep drag activation owned by BlockCanvas and prevent
BlockShell’s useDraggable path from mounting when rendered outside its
DndContext.
---
Nitpick comments:
In `@apps/console/src/lib/block-collision.test.ts`:
- Around line 4-7: Add behavioral tests around createBlockCollisionDetection,
covering accepted-container priority, filtering rejected descriptors, selecting
the correct nesting depth, and geometric fallback. Keep the existing factory
return-type assertion, and verify each case’s selected collision target rather
than only checking that the factory returns a function.
🪄 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 Plus
Run ID: af960cd4-7d0a-4a5f-a838-6900de34955b
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (52)
apps/console/e2e/console-sidebar.spec.tsapps/console/e2e/island-stripe-promotion.spec.tsapps/console/e2e/signatures.spec.tsapps/console/package.jsonapps/console/scripts/check-block-classes.mjsapps/console/src/app/api/harness/boot/route.tsapps/console/src/app/api/harness/plan/route.tsapps/console/src/components/blocks/BlockArrangementHost.tsxapps/console/src/components/blocks/BlockCanvas.tsxapps/console/src/components/blocks/BlockShell.test.tsxapps/console/src/components/blocks/BlockShell.tsxapps/console/src/components/blocks/IslandArrangementHost.tsxapps/console/src/components/blocks/IslandGrid.tsxapps/console/src/components/blocks/kind-glyph.tsxapps/console/src/components/shell/IntuiShell.tsxapps/console/src/components/shell/MainToolbar.tsxapps/console/src/components/shell/Sidebar.tsxapps/console/src/components/shell/StatusBar.tsxapps/console/src/components/shell/ViewInstanceHost.tsxapps/console/src/lib/block-collision.test.tsapps/console/src/lib/block-collision.tsapps/console/src/lib/block-geometry.test.tsapps/console/src/lib/block-geometry.tsapps/console/src/lib/block-move-receipts.tsapps/console/src/lib/block-placement.test.tsapps/console/src/lib/block-placement.tsapps/console/src/lib/console-host.test.tsapps/console/src/lib/console-host.tsapps/console/src/lib/island-grid.test.tsapps/console/src/lib/island-grid.tsapps/console/src/lib/island-promotion.test.tsapps/console/src/lib/island-promotion.tsapps/console/src/lib/state/proactivity-state.tsapps/console/src/lib/state/store-facade.tsapps/console/src/views/CardView.tsxapps/console/src/views/blocks/AutomationHistoryView.tsxapps/console/src/views/blocks/BlockEmptyBody.tsxapps/console/src/views/blocks/DeclaredBlocks.tsxapps/console/src/views/blocks/KanbanBlock.tsxapps/console/src/views/registry.tsxapps/console/src/views/workspace/WorkspaceSubstrateView.tsxdocs/plans/console/one-block-model/README.mddocs/plans/console/one-block-model/implementation-plan.mdpackages/block-view/package.jsonpackages/block-view/src/block-class.test.tspackages/block-view/src/block-class.tspackages/block-view/src/index.tspackages/block-view/src/island-class.test.tspackages/block-view/src/island-class.tspackages/block-view/src/registry.test.tspackages/block-view/src/registry.tspackages/block-view/src/types.ts
💤 Files with no reviewable changes (10)
- apps/console/src/lib/island-promotion.test.ts
- apps/console/src/lib/island-grid.test.ts
- packages/block-view/src/island-class.ts
- apps/console/src/components/blocks/IslandArrangementHost.tsx
- apps/console/src/components/shell/MainToolbar.tsx
- packages/block-view/src/island-class.test.ts
- apps/console/src/lib/island-promotion.ts
- apps/console/src/components/shell/StatusBar.tsx
- apps/console/src/components/blocks/IslandGrid.tsx
- apps/console/src/lib/island-grid.ts
| "gate:blocks": "node scripts/check-block-classes.mjs", | ||
| "tokens:manifest": "node scripts/generate-token-manifest.mjs", | ||
| "icons:normalize": "node scripts/normalize-noun-icons.mjs", | ||
| "generate:galley": "node scripts/generate-galley-register.mjs", | ||
| "gates": "npm run gate:fence && npm run gate:register && npm run gate:contrast && npm run gate:motion && npm run gate:icons && npm run gate:tokens && npm run gate:islands", | ||
| "gates": "npm run gate:fence && npm run gate:register && npm run gate:contrast && npm run gate:motion && npm run gate:icons && npm run gate:tokens && npm run gate:blocks", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
rg -n '"gates"|"test:e2e"|npm run gates|npm run test:e2e' apps/console/package.json .github 2>/dev/nullRepository: Travis-Gilbert/CommonPlace
Length of output: 432
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- apps/console/package.json ---\n'
cat -n apps/console/package.json | sed -n '1,120p'
printf '\n--- github references ---\n'
rg -n 'npm run (gates|test:e2e)|"gates"|"test:e2e"|gate:fence|gate:register|gate:contrast|gate:motion|gate:icons' .github apps/console/package.jsonRepository: Travis-Gilbert/CommonPlace
Length of output: 255
Include test:e2e in gates
npm run gates ends at gate:blocks, so it can pass without npm run test:e2e. Add the e2e run here, or make sure CI invokes it separately.
🤖 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 `@apps/console/package.json` around lines 21 - 25, Update the “gates” script in
package.json to include the existing “test:e2e” command, ensuring npm run gates
executes end-to-end tests in addition to the current gate checks.
Source: Coding guidelines
| const onResizeStart = useCallback( | ||
| (edge: ResizeEdge, event: ReactPointerEvent<HTMLButtonElement>) => { | ||
| if (!onGeometryChange) return; | ||
| const startX = event.clientX; | ||
| const startY = event.clientY; | ||
| const start = item.geometry; | ||
| const cell = cellRef.current; | ||
| const colW = cell ? cell.getBoundingClientRect().width / Math.max(1, start.colSpan) : 80; | ||
| const rowH = BLOCK_ROW_UNIT_PX; | ||
|
|
||
| const onMove = (moveEvent: PointerEvent) => { | ||
| const dCol = Math.round((moveEvent.clientX - startX) / colW); | ||
| const dRow = Math.round((moveEvent.clientY - startY) / rowH); | ||
| if (dCol === 0 && dRow === 0) return; | ||
| const next = clampGeometry(applyEdgeDelta(start, edge, dCol, dRow), limits); | ||
| onGeometryChange(item.viewInstanceId, next); | ||
| }; | ||
| const onUp = () => { | ||
| window.removeEventListener('pointermove', onMove); | ||
| window.removeEventListener('pointerup', onUp); | ||
| }; | ||
| window.addEventListener('pointermove', onMove); | ||
| window.addEventListener('pointerup', onUp); | ||
| }, | ||
| [item.geometry, item.viewInstanceId, limits, onGeometryChange], | ||
| ); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Resize drag fires an unthrottled persist call per pointer-move and leaks its listeners on unmount.
onMove calls onGeometryChange (which fires host.emit(...) fire-and-forget in BlockArrangementHost) on every rounded movement step with no debounce, so rapid intermediate writes can complete out of order and leave a stale geometry persisted. The pointermove/pointerup listeners are also only removed in onUp; if the cell unmounts mid-drag they're never cleaned up, leaking the closure and continuing to emit for a removed instance.
🔧 Proposed fix: commit once on pointerup, clean up on unmount
+ const activeResizeCleanup = useRef<(() => void) | null>(null);
+ useEffect(() => () => activeResizeCleanup.current?.(), []);
+
const onResizeStart = useCallback(
(edge: ResizeEdge, event: ReactPointerEvent<HTMLButtonElement>) => {
if (!onGeometryChange) return;
const startX = event.clientX;
const startY = event.clientY;
const start = item.geometry;
const cell = cellRef.current;
const colW = cell ? cell.getBoundingClientRect().width / Math.max(1, start.colSpan) : 80;
const rowH = BLOCK_ROW_UNIT_PX;
+ let latest = start;
const onMove = (moveEvent: PointerEvent) => {
const dCol = Math.round((moveEvent.clientX - startX) / colW);
const dRow = Math.round((moveEvent.clientY - startY) / rowH);
if (dCol === 0 && dRow === 0) return;
- const next = clampGeometry(applyEdgeDelta(start, edge, dCol, dRow), limits);
- onGeometryChange(item.viewInstanceId, next);
+ latest = clampGeometry(applyEdgeDelta(start, edge, dCol, dRow), limits);
};
const onUp = () => {
window.removeEventListener('pointermove', onMove);
window.removeEventListener('pointerup', onUp);
+ activeResizeCleanup.current = null;
+ onGeometryChange(item.viewInstanceId, latest);
};
+ activeResizeCleanup.current = onUp;
window.addEventListener('pointermove', onMove);
window.addEventListener('pointerup', onUp);
},
[item.geometry, item.viewInstanceId, limits, onGeometryChange],
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const onResizeStart = useCallback( | |
| (edge: ResizeEdge, event: ReactPointerEvent<HTMLButtonElement>) => { | |
| if (!onGeometryChange) return; | |
| const startX = event.clientX; | |
| const startY = event.clientY; | |
| const start = item.geometry; | |
| const cell = cellRef.current; | |
| const colW = cell ? cell.getBoundingClientRect().width / Math.max(1, start.colSpan) : 80; | |
| const rowH = BLOCK_ROW_UNIT_PX; | |
| const onMove = (moveEvent: PointerEvent) => { | |
| const dCol = Math.round((moveEvent.clientX - startX) / colW); | |
| const dRow = Math.round((moveEvent.clientY - startY) / rowH); | |
| if (dCol === 0 && dRow === 0) return; | |
| const next = clampGeometry(applyEdgeDelta(start, edge, dCol, dRow), limits); | |
| onGeometryChange(item.viewInstanceId, next); | |
| }; | |
| const onUp = () => { | |
| window.removeEventListener('pointermove', onMove); | |
| window.removeEventListener('pointerup', onUp); | |
| }; | |
| window.addEventListener('pointermove', onMove); | |
| window.addEventListener('pointerup', onUp); | |
| }, | |
| [item.geometry, item.viewInstanceId, limits, onGeometryChange], | |
| ); | |
| const activeResizeCleanup = useRef<(() => void) | null>(null); | |
| useEffect(() => () => activeResizeCleanup.current?.(), []); | |
| const onResizeStart = useCallback( | |
| (edge: ResizeEdge, event: ReactPointerEvent<HTMLButtonElement>) => { | |
| if (!onGeometryChange) return; | |
| const startX = event.clientX; | |
| const startY = event.clientY; | |
| const start = item.geometry; | |
| const cell = cellRef.current; | |
| const colW = cell ? cell.getBoundingClientRect().width / Math.max(1, start.colSpan) : 80; | |
| const rowH = BLOCK_ROW_UNIT_PX; | |
| let latest = start; | |
| const onMove = (moveEvent: PointerEvent) => { | |
| const dCol = Math.round((moveEvent.clientX - startX) / colW); | |
| const dRow = Math.round((moveEvent.clientY - startY) / rowH); | |
| if (dCol === 0 && dRow === 0) return; | |
| latest = clampGeometry(applyEdgeDelta(start, edge, dCol, dRow), limits); | |
| }; | |
| const onUp = () => { | |
| window.removeEventListener('pointermove', onMove); | |
| window.removeEventListener('pointerup', onUp); | |
| activeResizeCleanup.current = null; | |
| onGeometryChange(item.viewInstanceId, latest); | |
| }; | |
| activeResizeCleanup.current = onUp; | |
| window.addEventListener('pointermove', onMove); | |
| window.addEventListener('pointerup', onUp); | |
| }, | |
| [item.geometry, item.viewInstanceId, limits, onGeometryChange], | |
| ); |
🤖 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 `@apps/console/src/components/blocks/BlockCanvas.tsx` around lines 189 - 214,
Update onResizeStart so pointermove only tracks the latest clamped geometry and
does not call onGeometryChange for intermediate steps; commit the final geometry
once from the pointerup handler. Ensure pointermove and pointerup listeners are
removed both after pointerup and when the component unmounts, using the
component’s existing effect/lifecycle cleanup mechanism and preserving the
current resize calculations.
| export function resizeBlockAction( | ||
| viewInstanceId: string, | ||
| size: BlockSize, | ||
| geometry?: BlockGeometry, | ||
| ): ObjectAction { | ||
| return updateViewInstanceConfigAction(viewInstanceId, { | ||
| size, | ||
| ...(geometry ? { geometry } : {}), | ||
| }); | ||
| } | ||
|
|
||
| export function setBlockGeometryAction( | ||
| viewInstanceId: string, | ||
| geometry: BlockGeometry, | ||
| ): ObjectAction { | ||
| return updateViewInstanceConfigAction(viewInstanceId, { geometry }); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate relevant symbols
rg -n "updateViewInstanceConfigAction|resizeBlockAction|setBlockGeometryAction|nestBlockInContainerActions|onGeometryChange|placeBlockAction" apps/console packages -S
# Inspect the implementation of updateViewInstanceConfigAction if present in repo
fd -a "surface-actions" packages apps 2>/dev/null || true
fd -a "block-view" packages apps 2>/dev/null || trueRepository: Travis-Gilbert/CommonPlace
Length of output: 3895
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the implementation and callers with concise context.
rg -n -A 4 -B 4 "updateViewInstanceConfigAction|resizeBlockAction|setBlockGeometryAction|nestBlockInContainerActions|onGeometryChange|placeBlockAction" apps/console packages -SRepository: Travis-Gilbert/CommonPlace
Length of output: 21840
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Narrow inspection to the most likely implementation files for the config action.
fd -a "surface-actions" .
fd -a "block-placement.ts" apps/console
fd -a "BlockArrangementHost.tsx" apps/consoleRepository: Travis-Gilbert/CommonPlace
Length of output: 446
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== updateViewInstanceConfigAction references =="
rg -n -A 3 -B 3 "updateViewInstanceConfigAction" apps/console packages -S || true
echo
echo "== resize/set geometry callers =="
rg -n -A 3 -B 3 "resizeBlockAction|setBlockGeometryAction|onGeometryChange|placeBlockAction" apps/console -S || true
echo
echo "== likely implementation files =="
fd -a "surface-actions" apps packages 2>/dev/null || true
fd -a "block-placement.ts" apps/console 2>/dev/null || true
fd -a "BlockArrangementHost.tsx" apps/console 2>/dev/null || trueRepository: Travis-Gilbert/CommonPlace
Length of output: 16660
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the config action implementation and nearby types.
sed -n '1,140p' packages/block-view/src/surface-actions.ts
printf '\n====\n'
sed -n '1,220p' packages/block-view/src/block-view-types.tsRepository: Travis-Gilbert/CommonPlace
Length of output: 2680
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant tests around block placement behavior.
sed -n '1,220p' apps/console/src/lib/block-placement.test.tsRepository: Travis-Gilbert/CommonPlace
Length of output: 2540
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the action implementation and the related tests in one pass.
sed -n '1,180p' packages/block-view/src/surface-actions.ts
printf '\n==== TESTS ====\n'
sed -n '1,220p' apps/console/src/lib/block-placement.test.tsRepository: Travis-Gilbert/CommonPlace
Length of output: 4980
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether config updates are handled as patches or replacements in the reducer/store.
rg -n -A 4 -B 4 "UPDATE_VIEW_INSTANCE_CONFIG|viewInstanceConfig|config:" packages/block-view apps/console -SRepository: Travis-Gilbert/CommonPlace
Length of output: 11792
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the reducer/handler for config updates to determine merge vs replace semantics.
rg -n -A 6 -B 6 "UPDATE_VIEW_INSTANCE_CONFIG|updateViewInstanceConfigAction|viewInstanceConfig" packages apps -SRepository: Travis-Gilbert/CommonPlace
Length of output: 7194
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the exact implementation of the action creator.
sed -n '1,120p' packages/block-view/src/surface-actions.tsRepository: Travis-Gilbert/CommonPlace
Length of output: 2587
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the update handler/reducer that applies ObjectAction patches.
rg -n -A 8 -B 8 "patch:\s*\{\s*config|kind:\s*'update'|ObjectAction" packages apps -S
printf '\n====\n'
# Look for code that explicitly merges object config on update.
rg -n -A 6 -B 6 "config.*\.\.\.|Object.assign\(|merge.*config|patch\.config|properties\.config" packages apps -SRepository: Travis-Gilbert/CommonPlace
Length of output: 50383
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the code that applies update patches for view-instance objects.
rg -n -A 12 -B 12 "view-instance|patch\.config|patch:\s*\{[^}]*config|kind:\s*'update'" packages/block-view packages/block-view-contracts apps/console apps/web -S | head -n 300
printf '\n====\n'
# Specifically look for config merge/replacement logic near object update handling.
rg -n -A 8 -B 8 "config.*(merge|assign|spread)|Object\.assign\(|\.\.\..*config|properties\.config =|config\s*=\s*patch\.config" packages/block-view packages/block-view-contracts apps/console apps/web -S | head -n 300Repository: Travis-Gilbert/CommonPlace
Length of output: 50384
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search the repo for the code path that applies update patches to objects/config.
rg -n -A 6 -B 6 "patch\.config|Object\.assign\(|Object.fromEntries\(|config\s*=\s*|config:\s*config|patch:\s*\{" packages apps/commonplace-api apps/web apps/console -S | head -n 250
printf '\n====\n'
# Focus on any server-side object mutation code that mentions view-instance or config.
rg -n -A 8 -B 8 "view-instance|config.*patch|patch.*config|update.*config|ObjectAction" apps/commonplace-api packages/block-view apps/web -S | head -n 250Repository: Travis-Gilbert/CommonPlace
Length of output: 39939
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect MemoryBlockHost patch semantics.
sed -n '150,260p' packages/block-view/src/host/MemoryBlockHost.ts
printf '\n====\n'
rg -n -A 8 -B 8 "private patch|patch\\(o, action\\.patch\\)|patch\\(.*action\\.patch|function patch\\(" packages/block-view/src/host/MemoryBlockHost.ts packages/block-view/src/host -SRepository: Travis-Gilbert/CommonPlace
Length of output: 8604
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for view-instance update handling in the backend.
rg -n -A 8 -B 8 "view-instance|descriptor_id|patch.*config|config.*patch|update.*view" apps/commonplace-api crates packages -S | head -n 250
printf '\n====\n'
# Focus on Rust source files that may mutate object data.
fd -e rs apps/commonplace-api crates 2>/dev/null | sed -n '1,120p'Repository: Travis-Gilbert/CommonPlace
Length of output: 20693
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the Rust update handler that applies ObjectAction::Update and any config-specific merge logic.
rg -n -A 10 -B 10 "ObjectAction::.*update|kind:\s*'update'|patch\.config|config.*patch|update.*patch|with_extra\(\"config\"|extra\.get\(\"config\"\)" crates/commonplace crates/commonplace-api -S | head -n 250
printf '\n====\n'
# Read the most likely store implementation around update handling.
rg -n -A 12 -B 12 "ObjectAction::Update|Update.*patch|patch.*config" crates/commonplace/src/block_view.rs crates/commonplace/src -S | head -n 250Repository: Travis-Gilbert/CommonPlace
Length of output: 1843
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for the actual patch helper in the Rust block-view store.
rg -n -A 8 -B 8 "fn patch|patch\(&self|patch\(.*DbObject|patch.*DbObject|patch.*Object" crates/commonplace/src/block_view.rs crates/commonplace/src -S | head -n 250
printf '\n====\n'
# Search for any config field handling in the Rust store.
rg -n -A 6 -B 6 '"config"|config' crates/commonplace/src/block_view.rs crates/commonplace/src -S | head -n 250Repository: Travis-Gilbert/CommonPlace
Length of output: 20931
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the actual update path in the Rust store.
rg -n -A 12 -B 12 "pub fn emit_object_action|ObjectAction::Update|graph_transform.*Item.patch|fn patch\(|Item::patch|applyPatch" crates/commonplace/src/block_view.rs -S | head -n 250
printf '\n====\n'
# Read the surrounding implementation if the first search finds line numbers.
sed -n '2060,2205p' crates/commonplace/src/block_view.rsRepository: Travis-Gilbert/CommonPlace
Length of output: 10769
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the patch helper used by ObjectAction::Update.
rg -n -A 16 -B 16 "fn apply_item_patch|apply_item_patch\(" crates/commonplace/src/block_view.rs -S
printf '\n====\n'
# Read the helper body if the previous search locates it.
sed -n '780,860p' crates/commonplace/src/block_view.rsRepository: Travis-Gilbert/CommonPlace
Length of output: 5408
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the remainder of apply_item_patch to determine whether unknown keys are preserved.
sed -n '1721,1785p' crates/commonplace/src/block_view.rs
printf '\n====\n'
# If config is a dedicated field, locate its handling inside the same helper.
rg -n -A 4 -B 4 '"config"|config' crates/commonplace/src/block_view.rs -S | head -n 120Repository: Travis-Gilbert/CommonPlace
Length of output: 2734
Preserve existing view-instance config on resize and geometry updates.
updateViewInstanceConfigAction writes the whole config object, so resizeBlockAction and setBlockGeometryAction will overwrite sibling keys like kanbanColumn unless they merge the current config first.
🤖 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 `@apps/console/src/lib/block-placement.ts` around lines 46 - 62, Update
resizeBlockAction and setBlockGeometryAction to merge the existing view-instance
config before applying size or geometry changes, preserving sibling keys such as
kanbanColumn. Reuse the established current-config lookup and ensure
updateViewInstanceConfigAction receives the merged configuration rather than a
partial object.
| return this.writeThroughLayoutUpdates([ | ||
| { kind: 'update', id: action.id, patch }, | ||
| ]).then(() => applied([action.id])); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Serialize config write-through updates.
Line 761 starts an independent write for each update. Geometry changes are emitted from pointer movement without awaiting, so stale merged config snapshots can reach persistence after newer resize or nesting updates and overwrite newer fields. Coalesce or serialize updates per view instance, and test reversed completion order.
🤖 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 `@apps/console/src/lib/console-host.ts` around lines 761 - 763, Serialize or
coalesce the write-through operations initiated by the action-handling path
around writeThroughLayoutUpdates so each view instance persists updates in
order. Ensure newer resize or nesting patches cannot be overwritten by stale
config snapshots when pointer-driven updates complete out of order, and add
coverage that exercises reversed completion order.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
Add missing MailConnectView stubs, ObjectSet.subscribe on empty sets, and JsonValue casts for geometry config patches. Co-authored-by: Travis Gilbert <Travis-Gilbert@users.noreply.github.com>
Summary by CodeRabbit
New Features
Bug Fixes