Make Data-model full-bleed OWOX (retire plan-id ModelView chrome) - #179
Make Data-model full-bleed OWOX (retire plan-id ModelView chrome)#179Travis-Gilbert wants to merge 1 commit into
Conversation
Retire plan-id BlockShell, inspector, and fields/records lenses from ModelView so /Data-model is ModelCanvasShell edge-to-edge with a floating OKF/Propose/Diff/Restore strip; align the e2e proportions oracle.
|
Warning Review limit reached
Next review available in: 35 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
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 |
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
There was a problem hiding this comment.
Pull request overview
This PR simplifies the /Data-model surface by removing the plan-id “studio chrome” (BlockShell + inspector + non-diagram lenses) and making the OWOX-based canvas the full-bleed page body, while keeping ModelView as the registry adapter (load/pin/OKF/layout + floating action strip).
Changes:
- Convert
ModelViewfrom a multi-lens studio layout into a full-bleed canvas page with a floating OKF/Propose/Diff/Restore strip. - Wrap the diagram/canvas components to enforce full-height/full-width sizing for the page body.
- Update the Data-model e2e proportions check and cutover EXECUTE-REPORT note to reflect removal of the inspector rail.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/plans/commonplace-production-cutover/EXECUTE-REPORT.md | Updates cutover “honesty” note to reflect ModelView’s new role. |
| apps/console/src/views/model/ObservedDeclaredLenses.tsx | Makes DiagramLens explicitly full-bleed and adds a stable body hook. |
| apps/console/src/views/model/ModelView.tsx | Removes BlockShell/inspector/lenses and introduces full-bleed canvas page + floating strip. |
| apps/console/src/views/model/diagram/ForkDiagramCanvas.tsx | Ensures ModelCanvasShell can reliably fill available space. |
| apps/console/src/app/Data-model/page.tsx | Updates server-stamped comment to match the new full-bleed surface framing. |
| apps/console/e2e/console-sidebar.spec.ts | Updates the Data-model route check to assert no inspector rail and presence of full-bleed canvas page. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import { DiffDialog, diffGraphs, type ModelGraph } from '@commonplace/model-canvas'; | ||
| import { BlockShell } from '@/components/block/BlockShell'; | ||
| import { degradationFor, withAction } from '@/lib/degradation'; | ||
| import '@commonplace/model-canvas/canvas.css'; |
| <button | ||
| type="button" | ||
| onClick={() => setProposalComposerOpen((open) => !open)} | ||
| className="h-ij-control rounded-ij-arc px-3 text-ij-ink hover:bg-ij-hover-surface" | ||
| > | ||
| Propose | ||
| </button> |
| <label className="flex h-ij-control cursor-pointer items-center rounded-ij-arc px-3 text-ij-ink hover:bg-ij-hover-surface"> | ||
| Import OKF | ||
| <input | ||
| type="file" | ||
| accept=".md,.json,text/markdown,application/json" | ||
| multiple | ||
| className="sr-only" | ||
| onChange={(event) => void previewOkfImport(event.target.files)} | ||
| /> | ||
| </label> |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 86091cbf67
ℹ️ 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".
| Loading observed model. | ||
| </div> | ||
| ) : ( | ||
| <DiagramLens {...lensProps} /> |
There was a problem hiding this comment.
Restore field editing in the full-bleed canvas
When a user selects a declared field, the remaining DiagramLens path only records that selection in queryState; this commit deletes applyFieldEdit, the declaration call, and the only editor that consumed the selection, while ModelCanvasShell provides no replacement edit callback. Consequently the canonical MF4 field-edit flow is no longer available from /Data-model; add an in-canvas field editor wired to the registry mutation door before removing the inspector.
Useful? React with 👍 / 👎.
| Loading observed model. | ||
| </div> | ||
| ) : ( | ||
| <DiagramLens {...lensProps} /> |
There was a problem hiding this comment.
Expose unpin on the remaining canvas
After a ghost is declared, there is now no way to unpin it from the UI: this is the only rendered lens, but ForkDiagramCanvas does not consume onUnpin and ModelCanvasShell has no unpin callback. The retained applyUnpin function is therefore unreachable, so the commit's pin/unpin acceptance scenario cannot pass; provide an unpin affordance on declared canvas cards.
Useful? React with 👍 / 👎.
| ) : null} | ||
| <div | ||
| className="relative h-full min-h-0 bg-ij-editor" | ||
| data-model-canvas-page |
There was a problem hiding this comment.
Update the deployed acceptance selector
When THEOREM_RUN_CANVAS_LIVE=1, e2e/model-program-canvas.live.spec.ts:34 still waits for [data-model-studio], but this replacement root only emits data-model-canvas-page; a repo-wide search finds no remaining producer of the old attribute. The deployed Model Canvas acceptance test will therefore time out even when the new surface mounts, so update the live test or retain the compatibility marker.
Useful? React with 👍 / 👎.
| // @xyflow/react and tablecn structure through the registered lens components. | ||
| // SOURCING: @commonplace/model-canvas (OWOX hard fork) as the Data-model page | ||
| // body. Registry read/write stays in this adapter; plan-id BlockShell chrome | ||
| // is gone — the canvas is the page (SPEC-COMMONPLACE-MODEL-CANVAS-FORK-1.0). |
There was a problem hiding this comment.
Replace the em dash in this new source comment with allowed punctuation; the console constitution explicitly bans em and en dashes everywhere, including code comments.
AGENTS.md reference: apps/console/AGENTS.md:L169-L172
Useful? React with 👍 / 👎.
| {okfPreview ? ( | ||
| <div className="absolute bottom-3 left-3 right-3 z-30 mx-auto max-w-2xl rounded-ij-arc border border-ij-control-border bg-ij-selection/95 p-3 shadow-lg backdrop-blur" aria-label="OKF import preview"> |
There was a problem hiding this comment.
Make the bottom overlays mutually exclusive
If an OKF preview is open and the user opens or accepts a schema proposal, both states remain populated and both panels render at bottom-3 with z-30; the later OKF panel overlaps the proposal panel and can cover its Accept/Decline controls. These panels previously occupied separate flow sections, so either stack them or clear/disable the competing state when opening one.
Useful? React with 👍 / 👎.
| ) : null} | ||
| </div> | ||
|
|
||
| <div className="pointer-events-auto flex flex-wrap items-center gap-1 rounded-ij-arc border border-ij-control-border bg-ij-chrome/95 p-1 shadow-sm backdrop-blur"> |
There was a problem hiding this comment.
Back the floating action strip with a ledger source
The new floating action strip is assembled directly from a container plus native labels, buttons, and selects, but the console ledger has no row naming this model-canvas toolbar or its behavior; previously the surrounding shell need was supplied by BlockShell. Replace it with a ledger-backed component or add a named source row before introducing this bespoke surface.
AGENTS.md reference: apps/console/AGENTS.md:L87-L91
Useful? React with 👍 / 👎.
| : error | ||
| ? withAction(degradationFor(error, 500), () => setReloadToken((token) => token + 1)) | ||
| ? error |
There was a problem hiding this comment.
Keep wire errors out of the status banner
When /api/observed-model returns an error payload such as observed_model_graphql_unreachable, unavailableMessage now renders that value verbatim. The removed degradationFor(error, 500) call translated known codes to actionable sentences and hid unknown wire codes behind the generic unavailable message, so restore that normalization rather than exposing backend identifiers or arbitrarily long upstream error text to users.
Useful? React with 👍 / 👎.
Summary
BlockShell/ inspector / fields-records lenses fromModelView; the page body is full-bleedDiagramLens→ForkDiagramCanvas→ModelCanvasShell..commonplace-canonical.Test plan
/Data-modelsigned in: OWOX canvas fills the well; no inspector railapps/console/e2e/console-sidebar.spec.tsData-model proportions test (or live visual check)