fix(console): mount canonical canvas pages - #147
Conversation
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThe console now seeds a canonical Program Canvas surface, migrates existing layouts to bare canvas chrome, maps ChangesProgram Canvas integration
Sequence Diagram(s)sequenceDiagram
participant Browser
participant SurfaceRoutes
participant ConsoleHost
participant ConsoleShell
participant ProgramCanvas
Browser->>SurfaceRoutes: request /program
SurfaceRoutes->>ConsoleShell: activate console-program
ConsoleShell->>ConsoleHost: load canonical layout
ConsoleHost->>ConsoleShell: provide Program view and bare chrome
ConsoleShell->>ProgramCanvas: render program.canvas
Possibly related PRs
Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 mounts a new canonical Program canvas page as a durable console surface, updates routing to treat it as a page-owned surface, and migrates older persisted layouts so canonical canvas pages don’t get wrapped in extra shell chrome.
Changes:
- Add a seeded
console-programsurface (withprogram.canvasview instance) and include it in workspace seed + host migration paths. - Extend surface routing and routing-retirement tests to map
console-program↔/program. - Update shell rendering so editor regions with
chrome: 'bare'can render view instances without an extraBlockShell, plus add E2E coverage for the/programroute.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| apps/console/src/lib/workspace-seed.ts | Adds seeded Program surface/view-instance and marks canonical canvas editor regions as chrome: 'bare'. |
| apps/console/src/lib/view-routing-retirement.test.ts | Extends route ↔ surface-id mapping expectations for /program. |
| apps/console/src/lib/surface-routes.ts | Registers /program as a page-owned auxiliary surface route. |
| apps/console/src/lib/console-host.ts | Migrates persisted layouts to chrome: 'bare' for canonical canvas pages and writes through updates for durable layouts. |
| apps/console/src/lib/console-host.test.ts | Adds regression coverage for Program surface seeding and server-layout migration behavior. |
| apps/console/src/components/shell/EditorTabs.tsx | Passes bare rendering mode to ViewInstanceHost when a single-tab editor region is marked chrome: 'bare'. |
| apps/console/e2e/global-setup.ts | Prewarms /program route and includes it in shell-route readiness checks. |
| apps/console/e2e/console-sidebar.spec.ts | Adds Playwright acceptance that /program mounts console-program and renders the program canvas. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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: a5c7275100
ℹ️ 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".
| <ViewInstanceHost | ||
| instance={active} | ||
| host={host} | ||
| bare={bare && region.properties.chrome === 'bare'} |
There was a problem hiding this comment.
When a user adds a palette block or moves another view into the Models or Program editor, instances.length becomes greater than one, so this expression passes bare={false} even though the persisted region still declares chrome: 'bare'. Returning to the canonical canvas tab then wraps ProgramView or ModelView in the generic BlockShell again; ProgramView already renders its own BlockShell, producing the duplicate chrome this change is intended to remove. Keep tab-strip visibility separate from whether the region's view bodies are rendered bare.
Useful? React with 👍 / 👎.
| const shouldFillMissing = | ||
| missing.length > 0 && durable.length >= Math.ceil(seed.length / 2); |
There was a problem hiding this comment.
For any server layout containing the primary surface and landmarks, having only half of the current seed objects is enough to classify it as near-complete and recreate every missing surface, region, and view instance over HTTP. A deliberately reduced layout just above this cutoff is therefore expanded with almost half a seed, despite the adjacent contract saying sparse remotes must be adopted as-is; use a revision or known prior-seed signature rather than raw object count before modifying the durable arrangement.
AGENTS.md reference: apps/console/AGENTS.md:L153-L158
Useful? React with 👍 / 👎.
| if (additions.length > 0) { | ||
| await this.pushMissingSeedObjects(additions); |
There was a problem hiding this comment.
When a near-complete server layout is missing only a region or view instance while its seeded parent still exists, additions excludes that parent and this call creates only the missing node. pushMissingSeedObjects emits containment moves solely by iterating children of the objects it receives, while replaceLayout also leaves the surviving parent's old child list unchanged, so the recreated node remains orphaned and the surface still cannot render it. This also prevents recovery after a partially successful earlier seed push; derive the required parent-to-child moves from the full seed graph rather than only from additions.
AGENTS.md reference: apps/console/AGENTS.md:L153-L158
Useful? React with 👍 / 👎.
| layoutObject(PROGRAM_SURFACE_ID, 'surface', { | ||
| name: 'Program', kind: 'program', role: 'place', active: false, seed_revision: 1, | ||
| }, ['program.region-editor', ...companionIds('program')]), |
There was a problem hiding this comment.
The new Program surface is exposed in LayoutSwitcher because all seeded surfaces are passed to it, but LayoutSwitcher.switchTo only calls host.activateSurface and never navigates to the newly registered /program path. Selecting Program from the Layouts menu therefore leaves the previous URL in place; on reload, IntuiShell treats that pathname as authoritative and reactivates the previous surface, so the Program selection is lost. Route this new layout option through pathForSurfaceId and the router, as the launch-place navigation does.
Useful? React with 👍 / 👎.
Summary
Verification
Summary by CodeRabbit
New Features
/program.Bug Fixes
Tests