Consolidate CommonPlace architecture and graph-native search - #110
Conversation
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
|
Warning Review limit reached
Next review available in: 40 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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 ignored due to path filters (1)
📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR updates console navigation, seeded surfaces, shader rendering, graph filtering, reusable UI components, sourcing validation, API reconstruction forwarding, graph-store contracts, organizer filtering, and durable action receipt metadata. ChangesConsole experience and platform
API reconstruction and graph contracts
Contracts and repository tooling
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
Share layer selection helpers in @commonplace/multiplex-layers and filter ContextView edges with toolbar chips that match Console control chrome. Co-authored-by: Cursor <cursoragent@cursor.com>
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
![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: e7d593c61e
ℹ️ 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".
| return (await response.json()) as { aliases: AgentAliasRow[]; domain: string }; | ||
| } | ||
|
|
||
| export function AgentAliasPane({ userSlug = 'travis' }: { readonly userSlug?: string }) { |
There was a problem hiding this comment.
When any user opens the Rules view, it mounts this pane without a userSlug, so every list and mint request is issued as travis (and the route applies the same fallback). Consequently, signed-in users will see and create aliases in Travis's namespace rather than their own, despite the console already having a GitHub-backed session identity. Pass the verified session login/slug into the pane and reject requests that do not match it.
AGENTS.md reference: apps/console/AGENTS.md:L106-L106
Useful? React with 👍 / 👎.
| layoutObject('console-canvas', 'surface', { | ||
| name: 'Canvas', kind: 'canvas', role: 'place', stripe_order: 3, active: false, seed_revision: 1, | ||
| }, ['canvas.region-editor', ...companionIds('canvas')]), |
There was a problem hiding this comment.
For an account whose backend layout already contains console-chat and landmarks, ensureSeedLayoutOnce adopts the remote layout and only retains these new Canvas, Automation, and collection nodes as local-only objects; it never emits their creates or containment moves to the backend. They therefore appear in one browser but their activation and later layout edits target server IDs that do not exist, and another device cannot receive the new IA. Add a versioned remote-layout migration that creates these seed objects and their relations before accepting the remote snapshot.
AGENTS.md reference: apps/console/AGENTS.md:L132-L137
Useful? React with 👍 / 👎.
Load live NavItem objects from the host query and project object-kind entries into the Collections rail so declared types appear without a console code change. Co-authored-by: Cursor <cursoragent@cursor.com>
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d72df654e
ℹ️ 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".
| id: item.nav_item_id, | ||
| label, | ||
| path: `/records?type=${encodeURIComponent(objectTypeId)}`, | ||
| surfaceId: `console-records-${objectTypeId}`, |
There was a problem hiding this comment.
When a live NavItem produces a collection, this ID does not correspond to any seeded surface: ConsoleBlockHost.activateSurface rejects it, and the subsequent /records?type=... navigation activates only the generic console-records surface whose query is fixed to types: ['record']. Thus every dynamically registered object type either briefly leaves the current surface active or opens the unfiltered Records collection, so the new registry-driven destinations cannot display their declared type. Reuse a real parameterized surface or create and persist the generated surface and type-specific view instance.
AGENTS.md reference: apps/console/AGENTS.md:L132-L137
Useful? React with 👍 / 👎.
| const visibleEdges = useMemo(() => { | ||
| // Until the picker finishes loading, show every edge (fail open for first paint). | ||
| if (activeLayers.size === 0) return graph.edges; |
There was a problem hiding this comment.
After the user turns off the final layer chip, activeLayers.size becomes zero and this branch restores every graph edge instead of hiding them. The shared edgeVisibleForLayers helper explicitly treats an empty set as showing no edges, so the picker can never represent the valid “no layers” state. Track registry initialization separately from selection emptiness and only fail open before initialization.
Useful? React with 👍 / 👎.
| layoutObject('threads.vi-list', 'view-instance', { | ||
| descriptor_id: 'chat.surface', title: 'Threads', | ||
| query: { types: ['thread'] } as unknown as JsonValue, |
There was a problem hiding this comment.
Navigating to the new Threads collection mounts chat.surface, whose registered renderer is ThreadView at full composer density and does not consume this view instance's { types: ['thread'] } query. Consequently /threads is effectively another Chat surface rather than a browsable thread collection, despite the rail presenting it as one. Register and seed a descriptor that queries and lists thread objects, opening the chat surface only after a thread is selected.
AGENTS.md reference: apps/console/AGENTS.md:L132-L137
Useful? React with 👍 / 👎.
| return items | ||
| .filter((item) => item.kind.kind === 'object') |
There was a problem hiding this comment.
When a tenant contains NavItem objects scoped to different users, this projection includes every object-kind item without checking item.scope, even though the parser preserves each user_id. Since the sidebar passes the entire queried set here, one user can see another user's personal collections alongside workspace collections. Pass the verified session identity into the projection and retain only workspace items plus user items whose user_id matches it.
AGENTS.md reference: apps/console/AGENTS.md:L106-L106
Useful? React with 👍 / 👎.
| ) : headerDragListeners || actions || onHide || draggable ? ( | ||
| <div | ||
| ref={headerDragRef} | ||
| data-block-drag-surface={headerDragListeners ? 'true' : undefined} | ||
| className="absolute inset-x-0 top-0 z-10 flex h-6 items-center justify-end gap-1 px-2" |
There was a problem hiding this comment.
For a ground block whose size is full, BlockCanvas now hides the header and this absolutely positioned drag surface covers the entire top 24 pixels at a higher stacking level than the block body. Full-size renderers such as CanvasView place their Import and Export buttons in that top band, with the button center inside the overlay, so pointer clicks start a drag instead of reaching the controls. Use a dedicated drag handle or make the unused overlay area pointer-transparent.
Useful? React with 👍 / 👎.
| <section | ||
| aria-label="Landmarks" | ||
| aria-label="Pins" | ||
| data-rail-tier="pin" |
There was a problem hiding this comment.
When the tenant has any records, documents, or code files, useLandmarkObjects merges the pinned query with an unrestricted recent-items query, yet this renamed tier presents the entire result as Pins. Unpinned recent objects therefore appear as if the user pinned them, and choosing Unpin does not remove them because they remain in the recent query. Restrict this tier to pinned objects and persisted pinned view instances, or render recent items in a separately labeled tier.
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: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/console/src/views/ContextView.tsx (1)
233-242: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winEmpty-state message doesn't distinguish "no neighbors" from "filtered out by layers".
Once all layers are correctly hidden (see the fix above), this message will also show when the object genuinely has neighbors that the layer filter is hiding, which reads as if the object has none.
🤖 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/views/ContextView.tsx` around lines 233 - 242, Update the empty-state rendering in ContextView’s visibleEdges branch to distinguish between having no neighbors at all and having neighbors excluded by the active layer filter. Use the available full neighbor/edge collection and layer-filter state to show an appropriate filtered-out message when applicable, while preserving the existing no-neighbors message for genuinely empty selections.apps/console/src/components/material/ShaderSurface.tsx (1)
288-402: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy liftRecreating the WebGL mount on every uniform change churns contexts.
Every dep in this list tears the mount down and constructs a new
ShaderMount(new canvas, new WebGL context).Composer.tsxnow flipscolorBack,colorFill,paperKey, andstaticOnlyon plain focus/blur and on each run-state transition, so ordinary typing sessions repeatedly create and dispose contexts. Browsers cap live contexts and drop the oldest, which can push otherShaderSurfaceinstances into the static fallback (fallbackonly clears when a mount succeeds).Only
shader(andreduced) actually require a rebuild; the rest are already expressible throughmount.setUniforms/mount.setSpeed, which this file uses for theme changes.♻️ Suggested split: mount lifecycle vs uniform updates
- }, [ - colorBack, - colorFill, - colorStroke, - descriptor.amplitude, - dotGridKey, - gap, - paperKey, - reduced, - shader, - size, - staticOnly, - ]); + // Mount lifecycle only: the fragment program cannot change in place. + }, [shader]); + + // Uniform and speed updates reuse the live mount, so color, geometry, and + // composer-state changes never rebuild a WebGL context. + useEffect(() => { + const mount = mountRef.current; + if (!mount) return; + const paperParams = (JSON.parse(paperKey) as PaperSurfaceParams | null) ?? undefined; + const dotGridParams = (JSON.parse(dotGridKey) as DotGridSurfaceParams | null) ?? undefined; + mount.setUniforms(buildUniforms( + shader, + resolveCssColor(ijVar(colorBack)), + resolveCssColor(ijVar(colorFill)), + gap, + size, + descriptor.amplitude, + paperParams, + dotGridParams, + resolveCssColor(ijVar(colorStroke)), + )); + mount.setSpeed(reduced || staticOnly ? 0 : paperParams?.speed ?? 0.35); + }, [ + colorBack, + colorFill, + colorStroke, + descriptor.amplitude, + dotGridKey, + gap, + paperKey, + reduced, + shader, + size, + staticOnly, + ]);The mount effect then needs to read the latest color and param values through refs so its initial uniforms stay correct.
🤖 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/material/ShaderSurface.tsx` around lines 288 - 402, Split the effect in ShaderSurface into mount lifecycle and update responsibilities: keep ShaderMount creation and disposal dependent only on shader and reduced, while moving color, paperKey, dotGridKey, gap, size, descriptor.amplitude, and staticOnly updates through mount.setUniforms/setSpeed. Store the latest uniform inputs in refs so the stable mount effect builds correct initial uniforms, and ensure subsequent prop changes update the existing mount without recreating its WebGL context.
🧹 Nitpick comments (4)
apps/console/src/lib/rail/nav-registry.ts (1)
96-96: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider narrowing
kindinstead of casting.An unexpected shape (for example a plain string
'object'from the MCP registry) passes the cast and is then silently dropped bycollectionsFromNavRegistry, so a declared type just never appears with no signal. A small type guard would make the mismatch observable.♻️ Optional guard
- const kind = (props.kind ?? {}) as NavItemKind; + const rawKind = props.kind; + const kind = ( + rawKind && typeof rawKind === 'object' && 'kind' in rawKind ? rawKind : { kind: 'folder', name: '' } + ) as NavItemKind;🤖 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/rail/nav-registry.ts` at line 96, The kind extraction in the nav registry currently trusts props.kind via a cast, allowing invalid runtime values to be silently discarded by collectionsFromNavRegistry. Replace the cast around kind with a type guard that validates supported NavItemKind values and makes unexpected shapes observable, while preserving valid kind handling.packages/multiplex-layers/package.json (1)
9-12: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winPrefer local
tscovernpx -pfor the check script.
typescriptis already a pinned devDependency, sonpx --yes -p typescript@5.7.3 tsc --noEmitis redundant and adds an extra network install step (potential CI flakiness/slowdown) instead of using the already-resolved local binary.♻️ Proposed fix
- "check": "npx --yes -p typescript@5.7.3 tsc --noEmit", + "check": "tsc --noEmit",🤖 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 `@packages/multiplex-layers/package.json` around lines 9 - 12, Update the package check script to invoke the locally installed TypeScript compiler directly, removing the npx package-install invocation and its duplicated version pin while preserving the existing no-emit behavior.apps/console/src/components/context/LayerPicker.tsx (1)
26-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUnimplemented non-pinnable layer indicator (TODO).
markNonPinnableChipis a stub; non-pinnable layers (presence,semantic) currently only signal their status via thetitletooltip, which isn't reliably surfaced to keyboard/screen-reader users.Want me to implement a register-native badge (e.g. a muted underline using
--ij-*tokens) formarkNonPinnableChipand open a follow-up issue if you'd rather track it separately?Also applies to: 108-111
🤖 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/context/LayerPicker.tsx` around lines 26 - 35, Implement markNonPinnableChip to return a visible, accessible register-native indicator for non-pinnable layers instead of null. Use the existing --ij-* styling tokens and appropriate semantic text so keyboard and screen-reader users receive the status without relying on the title tooltip; preserve the component’s ReactNode return contract and avoid introducing a separate status language.apps/console/src/components/ground/MaterialLayer.tsx (1)
231-237: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winPer-island
getComputedStyleruns inside the paint loop.
paintis a RAF callback that the body-wideMutationObserver(which watchesstyleandclasson the whole subtree) can mark dirty every frame, so this adds up to 12 style resolutions per frame on top of the existing rect reads. Since the radius is driven by[data-block-size], reading it once per island geometry change (or resolving it fromnode.dataset.blockSizeagainst a small token map) keeps the loop rect-only.🤖 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/ground/MaterialLayer.tsx` around lines 231 - 237, Move the per-island getComputedStyle call out of the paint loop in MaterialLayer’s geometry handling, caching each island’s radius when its geometry or data-block-size changes. Reuse the cached radius during paint alongside the existing rect reads, or resolve node.dataset.blockSize through a small token map, while preserving the --ij-island-radius fallback behavior.
🤖 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/e2e/signatures.spec.ts`:
- Around line 150-153: Update the companion shortcut handling to match shifted
number keys using the physical event.code values (such as Digit1) rather than
event.key, while preserving the existing event.shiftKey branch and companion
toggle behavior for Alt+Shift shortcuts.
In `@apps/console/src/app/api/agent-address/aliases/route.ts`:
- Around line 10-58: Enforce authenticated-principal consistency in GET, POST,
and DELETE before calling listAgentAliases, mintAgentAlias, or revokeAgentAlias.
Validate supplied userSlug values against the authenticated primary user, reject
mismatches, require that primary user for DELETE, and pass the validated
principal through so aliases cannot be accessed or revoked across users.
In `@apps/console/src/components/blocks/BlockShell.tsx`:
- Around line 254-262: Update the header overlay rendered by BlockShell so its
full-width container does not intercept pointer events over the block body; keep
pointer events enabled only on the right-aligned controls and, when
headerDragListeners is present, provide a narrow fixed-width right-side drag
activation zone that preserves dragging without covering the first 24px of
content.
In `@apps/console/src/components/material/ShaderSurface.tsx`:
- Around line 404-422: Update the fallback rendering branch in ShaderSurface so
fluted-glass and dithering, like paper-texture and grain-gradient, do not
receive the radial-gradient dot grid. Preserve the existing background color,
sizing, opacity, and dot-grid behavior for other shaders.
In `@apps/console/src/components/shell/MainToolbar.tsx`:
- Around line 152-159: Update the group wrapper rendered by the groups.map
callback in MainToolbar to use role="group", while preserving its existing key,
data-layout-group, styling, and label content so the menu exposes each
Work/Objects/Tools/System section as a distinct group.
In `@apps/console/src/components/shell/Sidebar.tsx`:
- Around line 330-336: The pin-drop flow in the placeBlockAction call must
handle its synchronous ObjectAction[] result instead of treating it as a
Promise. Replace the .then callback with iteration that emits each returned
action through host.emit(action), then call recordBlockMoveReceipts with the
emitted actions’ length.
In `@apps/console/src/lib/rail/rail-model.test.ts`:
- Around line 48-50: Update the test case “rejects duplicate labels across
places and collections” to provide fixtures containing duplicate labels in both
places and collections, then invoke assertUniqueRailLabels with those fixtures
and assert that it throws. Keep the test focused on exercising the rejection
path rather than the production defaults.
- Around line 40-46: The test named “adding a temporary collection policy
surfaces a new entry” does not modify policy state and only verifies the
existing hidden Kanban behavior. Either inject a visible Kanban policy override
through the supported mechanism and assert the derived collection count/entry,
or rename the test to describe and validate the current hidden-policy behavior.
In `@apps/console/src/lib/workspace-seed.ts`:
- Around line 296-306: Prevent the Files surface from injecting the redundant
“files” companion, since files.vi-tree already uses descriptor_id “files.tree”
with the same query. Update the files layout setup and the
companionSeeds/companionIds flow as needed to exclude only this companion while
preserving other companion tools and existing behavior for all other surfaces.
In `@apps/console/src/styles/int-ui-register.css`:
- Around line 45-50: Update --ij-raised to reference the intended tier
explicitly: use the raised-tier value if raised surfaces should map to
--ij-tier-raised, or the floating-tier value if they intentionally map to
--ij-tier-floating. Align the surrounding comment with that choice so the
surface-to-tier mapping is clear.
In `@apps/console/src/views/ContextView.tsx`:
- Line 60: Update the layer-selection state and related logic in ContextView to
distinguish the initial “registry not yet loaded” state from an explicit empty
user selection. Use an explicit sentinel for the unloaded state, preserve
fail-open behavior only for that sentinel, and ensure a `{ layers: [] }` value
from LayerPicker causes visibleEdges to hide all edges rather than show every
edge; update the affected layerSelection checks and handlers consistently.
In `@docs/plans/console-information-architecture/implementation-plan.md`:
- Line 65: Complete all merge-blocking Console verification before updating the
plans: in docs/plans/console-information-architecture/implementation-plan.md at
lines 65-65, resolve the gate:register failure, including the AgentAliasPane
issue, and record a passing result; in
docs/plans/data-canvas-graph-native/report.md at lines 9-12, rerun the contrast,
motion, token, island, and full Console E2E gates and document their results.
In `@docs/plans/console/36-DESIGN-FIX-LADDER-MATERIAL-CHROME.md`:
- Around line 17-23: Update the plan’s Proof section to include the required
merge-blocking checks: gate:fence, gate:register, gate:contrast, gate:motion,
gate:icons, and Playwright visual baselines via test:e2e. Remove or revise the
Lines 27-29 language that defers the visual pass so the plan is not described as
proven until all listed gates pass.
In `@packages/block-view-contracts/src/block-view-types.ts`:
- Around line 218-232: Replace the optional op_range and legacy_without_op_range
fields in ObjectActionReceipt with a discriminated union: define a durable
variant requiring op_range and excluding the legacy marker, and a legacy variant
requiring legacy_without_op_range: true and excluding op_range. Keep the shared
receipt fields unchanged and ensure each valid receipt is explicitly classified
as durable or legacy.
---
Outside diff comments:
In `@apps/console/src/components/material/ShaderSurface.tsx`:
- Around line 288-402: Split the effect in ShaderSurface into mount lifecycle
and update responsibilities: keep ShaderMount creation and disposal dependent
only on shader and reduced, while moving color, paperKey, dotGridKey, gap, size,
descriptor.amplitude, and staticOnly updates through mount.setUniforms/setSpeed.
Store the latest uniform inputs in refs so the stable mount effect builds
correct initial uniforms, and ensure subsequent prop changes update the existing
mount without recreating its WebGL context.
In `@apps/console/src/views/ContextView.tsx`:
- Around line 233-242: Update the empty-state rendering in ContextView’s
visibleEdges branch to distinguish between having no neighbors at all and having
neighbors excluded by the active layer filter. Use the available full
neighbor/edge collection and layer-filter state to show an appropriate
filtered-out message when applicable, while preserving the existing no-neighbors
message for genuinely empty selections.
---
Nitpick comments:
In `@apps/console/src/components/context/LayerPicker.tsx`:
- Around line 26-35: Implement markNonPinnableChip to return a visible,
accessible register-native indicator for non-pinnable layers instead of null.
Use the existing --ij-* styling tokens and appropriate semantic text so keyboard
and screen-reader users receive the status without relying on the title tooltip;
preserve the component’s ReactNode return contract and avoid introducing a
separate status language.
In `@apps/console/src/components/ground/MaterialLayer.tsx`:
- Around line 231-237: Move the per-island getComputedStyle call out of the
paint loop in MaterialLayer’s geometry handling, caching each island’s radius
when its geometry or data-block-size changes. Reuse the cached radius during
paint alongside the existing rect reads, or resolve node.dataset.blockSize
through a small token map, while preserving the --ij-island-radius fallback
behavior.
In `@apps/console/src/lib/rail/nav-registry.ts`:
- Line 96: The kind extraction in the nav registry currently trusts props.kind
via a cast, allowing invalid runtime values to be silently discarded by
collectionsFromNavRegistry. Replace the cast around kind with a type guard that
validates supported NavItemKind values and makes unexpected shapes observable,
while preserving valid kind handling.
In `@packages/multiplex-layers/package.json`:
- Around line 9-12: Update the package check script to invoke the locally
installed TypeScript compiler directly, removing the npx package-install
invocation and its duplicated version pin while preserving the existing no-emit
behavior.
🪄 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: 17e16878-ca6b-4b8f-b625-245bf7d4120e
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (58)
apps/console/AGENTS.mdapps/console/CLAUDE.mdapps/console/e2e/appearance.spec.tsapps/console/e2e/console-ia.spec.tsapps/console/e2e/console-sidebar.spec.tsapps/console/e2e/signatures.spec.tsapps/console/package.jsonapps/console/scripts/check-contrast.mjsapps/console/src/app/api/agent-address/aliases/route.tsapps/console/src/app/api/layers/route.tsapps/console/src/app/automation/page.tsxapps/console/src/app/canvas/page.tsxapps/console/src/app/files/page.tsxapps/console/src/app/records/page.tsxapps/console/src/app/threads/page.tsxapps/console/src/components/agent-address/AgentAliasPane.tsxapps/console/src/components/blocks/BlockCanvas.tsxapps/console/src/components/blocks/BlockShell.test.tsxapps/console/src/components/blocks/BlockShell.tsxapps/console/src/components/composer/Composer.tsxapps/console/src/components/context/LayerPicker.tsxapps/console/src/components/ground/MaterialLayer.tsxapps/console/src/components/material/EmptyRegion.tsxapps/console/src/components/material/ShaderSurface.tsxapps/console/src/components/shell/EditorTabs.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/lib/canvas/store.tsapps/console/src/lib/console-host.test.tsapps/console/src/lib/material/materials.tsapps/console/src/lib/rail/nav-registry.tsapps/console/src/lib/rail/rail-model.test.tsapps/console/src/lib/rail/rail-model.tsapps/console/src/lib/server/agent-address-harness.tsapps/console/src/lib/surface-routes.tsapps/console/src/lib/workspace-seed.tsapps/console/src/motion/motion-tokens.tsapps/console/src/styles/app.cssapps/console/src/styles/int-ui-register.cssapps/console/src/styles/register-bridge.cssapps/console/src/styles/token-manifest.jsonapps/console/src/views/AppearanceView.tsxapps/console/src/views/ContextView.tsxapps/console/src/views/IndexRulesView.tsxapps/console/src/views/ViewStates.test.tsxapps/console/src/views/ViewStates.tsxapps/console/src/views/canvas/CanvasPaperGround.tsxapps/console/src/views/registry.tsxdocs/plans/console-information-architecture/implementation-plan.mddocs/plans/console/36-DESIGN-FIX-LADDER-MATERIAL-CHROME.mddocs/plans/data-canvas-graph-native/report.mdpackages/block-view-contracts/src/block-view-types.tspackages/multiplex-layers/package.jsonpackages/multiplex-layers/src/index.test.tspackages/multiplex-layers/src/index.tspackages/multiplex-layers/tsconfig.json
| if (fallback) { | ||
| const gapX = dotGrid?.gapX ?? gap; | ||
| const gapY = dotGrid?.gapY ?? gap; | ||
| return ( | ||
| <div | ||
| className={className} | ||
| aria-hidden | ||
| style={{ | ||
| ...style, | ||
| backgroundColor: ijVar(colorBack), | ||
| backgroundImage: `radial-gradient(circle, ${ijVar(colorFill)} 1px, transparent 1px)`, | ||
| backgroundSize: `${gap}px ${gap}px`, | ||
| backgroundImage: shader === 'paper-texture' || shader === 'grain-gradient' | ||
| ? undefined | ||
| : `radial-gradient(circle, ${ijVar(colorFill)} 1px, transparent 1px)`, | ||
| backgroundSize: `${gapX}px ${gapY}px`, | ||
| opacity: shader === 'paper-texture' ? 0.92 : 1, | ||
| }} | ||
| /> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fallback paints a dot grid for fluted-glass and dithering.
The exclusion list only covers paper-texture and grain-gradient, so a refused composer (fluted-glass) that hits the context budget renders a dotted pattern behind the input rather than a flat material.
🐛 Proposed fix
- backgroundImage: shader === 'paper-texture' || shader === 'grain-gradient'
- ? undefined
- : `radial-gradient(circle, ${ijVar(colorFill)} 1px, transparent 1px)`,
+ backgroundImage: shader === 'dot-grid'
+ ? `radial-gradient(circle, ${ijVar(colorFill)} 1px, transparent 1px)`
+ : undefined,📝 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.
| if (fallback) { | |
| const gapX = dotGrid?.gapX ?? gap; | |
| const gapY = dotGrid?.gapY ?? gap; | |
| return ( | |
| <div | |
| className={className} | |
| aria-hidden | |
| style={{ | |
| ...style, | |
| backgroundColor: ijVar(colorBack), | |
| backgroundImage: `radial-gradient(circle, ${ijVar(colorFill)} 1px, transparent 1px)`, | |
| backgroundSize: `${gap}px ${gap}px`, | |
| backgroundImage: shader === 'paper-texture' || shader === 'grain-gradient' | |
| ? undefined | |
| : `radial-gradient(circle, ${ijVar(colorFill)} 1px, transparent 1px)`, | |
| backgroundSize: `${gapX}px ${gapY}px`, | |
| opacity: shader === 'paper-texture' ? 0.92 : 1, | |
| }} | |
| /> | |
| ); | |
| } | |
| if (fallback) { | |
| const gapX = dotGrid?.gapX ?? gap; | |
| const gapY = dotGrid?.gapY ?? gap; | |
| return ( | |
| <div | |
| className={className} | |
| aria-hidden | |
| style={{ | |
| ...style, | |
| backgroundColor: ijVar(colorBack), | |
| backgroundImage: shader === 'dot-grid' | |
| ? `radial-gradient(circle, ${ijVar(colorFill)} 1px, transparent 1px)` | |
| : undefined, | |
| backgroundSize: `${gapX}px ${gapY}px`, | |
| opacity: shader === 'paper-texture' ? 0.92 : 1, | |
| }} | |
| /> | |
| ); | |
| } |
🤖 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/material/ShaderSurface.tsx` around lines 404 -
422, Update the fallback rendering branch in ShaderSurface so fluted-glass and
dithering, like paper-texture and grain-gradient, do not receive the
radial-gradient dot grid. Preserve the existing background color, sizing,
opacity, and dot-grid behavior for other shaders.
| {groups.map((group, groupIndex) => ( | ||
| <div key={group.id} data-layout-group={group.id} className={groupIndex > 0 ? 'mt-1 border-t border-ij-seam pt-1' : undefined}> | ||
| <div | ||
| className="px-2 py-1 font-ij-mono text-ij-island-meta text-ij-ink-info" | ||
| aria-hidden | ||
| > | ||
| {group.label} | ||
| </div> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Expose the menu groups with role="group".
Direct children of role="menu" must be menu items, groups, or separators. The plain <div> wrappers plus an aria-hidden label mean assistive tech sees one flat list and never hears the Work/Objects/Tools/System structure.
♿ Proposed fix
- <div key={group.id} data-layout-group={group.id} className={groupIndex > 0 ? 'mt-1 border-t border-ij-seam pt-1' : undefined}>
+ <div
+ key={group.id}
+ role="group"
+ aria-labelledby={`layout-group-${group.id}`}
+ data-layout-group={group.id}
+ className={groupIndex > 0 ? 'mt-1 border-t border-ij-seam pt-1' : undefined}
+ >
<div
+ id={`layout-group-${group.id}`}
className="px-2 py-1 font-ij-mono text-ij-island-meta text-ij-ink-info"
- aria-hidden
>📝 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.
| {groups.map((group, groupIndex) => ( | |
| <div key={group.id} data-layout-group={group.id} className={groupIndex > 0 ? 'mt-1 border-t border-ij-seam pt-1' : undefined}> | |
| <div | |
| className="px-2 py-1 font-ij-mono text-ij-island-meta text-ij-ink-info" | |
| aria-hidden | |
| > | |
| {group.label} | |
| </div> | |
| {groups.map((group, groupIndex) => ( | |
| <div | |
| key={group.id} | |
| role="group" | |
| aria-labelledby={`layout-group-${group.id}`} | |
| data-layout-group={group.id} | |
| className={groupIndex > 0 ? 'mt-1 border-t border-ij-seam pt-1' : undefined} | |
| > | |
| <div | |
| id={`layout-group-${group.id}`} | |
| className="px-2 py-1 font-ij-mono text-ij-island-meta text-ij-ink-info" | |
| > | |
| {group.label} | |
| </div> |
🤖 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/shell/MainToolbar.tsx` around lines 152 - 159,
Update the group wrapper rendered by the groups.map callback in MainToolbar to
use role="group", while preserving its existing key, data-layout-group, styling,
and label content so the menu exposes each Work/Objects/Tools/System section as
a distinct group.
| /* Structural surfaces mapped onto the tier axis (hex so gates can resolve). | ||
| Frame is lighter than both island fills (frame-inversion for dark). */ | ||
| --ij-frame: #1C1E22; | ||
| --ij-editor: #121314; | ||
| --ij-chrome: #17191C; | ||
| --ij-raised: #383B42; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
--ij-raised maps to the floating tier value, not --ij-tier-raised.
--ij-raised: #383B42`` is the --ij-tier-floating value; `--ij-tier-raised` (`#2A2D33`) ends up consumed only by `--ij-divider`. If raised surfaces (composer background, popovers via `bg-ij-raised`) are meant to sit on the floating step, say so with the variable so the ladder stays readable; otherwise this is one step off.
🐛 Proposed fix (if raised should be the raised tier)
- --ij-raised: `#383B42`;
+ --ij-raised: `#2A2D33`;Or, if the floating step is intended, reference it explicitly and note why in the comment above.
📝 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.
| /* Structural surfaces mapped onto the tier axis (hex so gates can resolve). | |
| Frame is lighter than both island fills (frame-inversion for dark). */ | |
| --ij-frame: #1C1E22; | |
| --ij-editor: #121314; | |
| --ij-chrome: #17191C; | |
| --ij-raised: #383B42; | |
| /* Structural surfaces mapped onto the tier axis (hex so gates can resolve). | |
| Frame is lighter than both island fills (frame-inversion for dark). */ | |
| --ij-frame: `#1C1E22`; | |
| --ij-editor: `#121314`; | |
| --ij-chrome: `#17191C`; | |
| --ij-raised: `#2A2D33`; |
🤖 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/styles/int-ui-register.css` around lines 45 - 50, Update
--ij-raised to reference the intended tier explicitly: use the raised-tier value
if raised surfaces should map to --ij-tier-raised, or the floating-tier value if
they intentionally map to --ij-tier-floating. Align the surrounding comment with
that choice so the surface-to-tier mapping is clear.
| const selectRecord = useShellStore((state) => state.selectRecord); | ||
| const memories = useMemoryProjectionStore((state) => state.items); | ||
| const [candidates, setCandidates] = useState<readonly ObjectRef[]>([]); | ||
| const [layerSelection, setLayerSelection] = useState<LayerSelectionState>({ layers: [] }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Deselecting all layers shows every edge instead of hiding them.
{layers: []} is used to mean both "registry not yet loaded" (line 60 initial state, intended fail-open) and "user cleared every layer" (reachable via toggleLayer deselecting the last active layer, since LayerPicker reports {layers: []} back through onLayerSetChange). visibleEdges's activeLayers.size === 0 check can't tell these apart, so a user who turns off every layer toggle will see all edges reappear, the opposite of the intended filter.
Distinguish "not yet loaded" from "user cleared selection" with an explicit sentinel instead of reusing array emptiness.
🐛 Proposed fix
- const [layerSelection, setLayerSelection] = useState<LayerSelectionState>({ layers: [] });
+ const [layerSelection, setLayerSelection] = useState<LayerSelectionState | null>(null);- const activeLayers = useMemo(() => new Set(layerSelection.layers), [layerSelection.layers]);
+ const activeLayers = useMemo(() => new Set(layerSelection?.layers ?? []), [layerSelection]);
const visibleEdges = useMemo(() => {
- // Until the picker finishes loading, show every edge (fail open for first paint).
- if (activeLayers.size === 0) return graph.edges;
+ // Until the picker finishes loading, show every edge (fail open for first paint).
+ if (layerSelection === null) return graph.edges;
return graph.edges.filter((edge) =>
edgeVisibleForLayers(layerIdForEdgeType(edge.relation), activeLayers),
);
- }, [activeLayers, graph.edges]);
+ }, [activeLayers, graph.edges, layerSelection]);Also applies to: 148-154
🤖 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/views/ContextView.tsx` at line 60, Update the
layer-selection state and related logic in ContextView to distinguish the
initial “registry not yet loaded” state from an explicit empty user selection.
Use an explicit sentinel for the unloaded state, preserve fail-open behavior
only for that sentinel, and ensure a `{ layers: [] }` value from LayerPicker
causes visibleEdges to hide all edges rather than show every edge; update the
affected layerSelection checks and handlers consistently.
| | D6 | Composer states | Six states on one ShaderSurface; Paper fragments (paper-texture / grain-gradient / fluted-glass); stream-driven motion | done | | ||
| | D7 | Connection | One owner in StatusBar | done | | ||
| | D8 | Empty causes | Default actions: reconnect / clear / loading | done | | ||
| | G | System | Unit tests + contrast/motion/blocks gates pass; register gate blocked by pre-existing AgentAliasPane | partial | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Complete the required Console gates before merge.
The plan records gate:register as blocked, while the status report says contrast, motion, token, island, and full Console E2E verification were not rerun. This cohort cannot satisfy its merge contract yet.
docs/plans/console-information-architecture/implementation-plan.md#L65-L65: resolve the register-gate failure, including the pre-existingAgentAliasPaneissue, then record a passing result.docs/plans/data-canvas-graph-native/report.md#L9-L12: run the remaining required gates and Console E2E suite, then update the verification status with their results.
As per coding guidelines, all merge-blocking gates must pass.
📍 Affects 2 files
docs/plans/console-information-architecture/implementation-plan.md#L65-L65(this comment)docs/plans/data-canvas-graph-native/report.md#L9-L12
🤖 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 `@docs/plans/console-information-architecture/implementation-plan.md` at line
65, Complete all merge-blocking Console verification before updating the plans:
in docs/plans/console-information-architecture/implementation-plan.md at lines
65-65, resolve the gate:register failure, including the AgentAliasPane issue,
and record a passing result; in docs/plans/data-canvas-graph-native/report.md at
lines 9-12, rerun the contrast, motion, token, island, and full Console E2E
gates and document their results.
Source: Coding guidelines
| - `npm run gate:contrast` | ||
| - `npm run gate:tokens` | ||
| - `npm run gate:radius` | ||
| - `npm run gate:icons` | ||
| - `npm run gate:fence` | ||
| - `npm run gate:blocks` | ||
| - `vitest` BlockShell + theme-engine |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Record the required visual and motion gates before calling the plan proven.
The proof list omits gate:motion, gate:register, and Playwright visual baselines, while Lines 27-28 explicitly defer the visual pass. Add those checks to Proof, or mark the plan as pending until they pass.
As per coding guidelines, all merge-blocking gates must pass: gate:fence, gate:register, gate:contrast, gate:motion, gate:icons, and Playwright visual baselines via test:e2e.
Also applies to: 27-29
🤖 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 `@docs/plans/console/36-DESIGN-FIX-LADDER-MATERIAL-CHROME.md` around lines 17 -
23, Update the plan’s Proof section to include the required merge-blocking
checks: gate:fence, gate:register, gate:contrast, gate:motion, gate:icons, and
Playwright visual baselines via test:e2e. Remove or revise the Lines 27-29
language that defers the visual pass so the plan is not described as proven
until all listed gates pass.
Source: Coding guidelines
| export interface ObjectActionReceipt { | ||
| readonly action_kind: ActionKind; | ||
| readonly status: ObjectActionStatus; | ||
| readonly target_ids?: readonly string[]; | ||
| readonly graph_transform?: string; | ||
| readonly actor_id?: string; | ||
| readonly note?: string; | ||
| /** | ||
| * Present on durable captured emits (`emit_object_action_captured`). | ||
| * Absent only on explicit legacy receipts; those must be labeled legacy | ||
| * and must not be treated as ledger-backed. | ||
| */ | ||
| readonly op_range?: OpRange; | ||
| /** True when this receipt was minted without an op range (legacy path). */ | ||
| readonly legacy_without_op_range?: boolean; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Make the receipt invariant type-safe.
op_range and legacy_without_op_range are independent optionals, so invalid or ambiguous receipts still compile. Model them as a discriminated union: durable receipts must contain op_range, while legacy receipts must explicitly set legacy_without_op_range: true.
🤖 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 `@packages/block-view-contracts/src/block-view-types.ts` around lines 218 -
232, Replace the optional op_range and legacy_without_op_range fields in
ObjectActionReceipt with a discriminated union: define a durable variant
requiring op_range and excluding the legacy marker, and a legacy variant
requiring legacy_without_op_range: true and excluding op_range. Keep the shared
receipt fields unchanged and ensure each valid receipt is explicitly classified
as durable or legacy.
|
Closing as superseded after review.
If any leftover (for example NavItem registry integration) is still wanted, salvage it in a fresh PR off current |
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0b538b15a4
ℹ️ 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".
| this.canvas = new CanvasStore( | ||
| options.canvasTenant === undefined ? tenant : options.canvasTenant, | ||
| (canvas) => { | ||
| // Live mode write-through: placements are ObjectActions, not localStorage. | ||
| if (this.records !== null) return; | ||
| const refs = graphToObjectRefs(canvas); | ||
| for (const ref of refs) { | ||
| void this.http.emit({ |
There was a problem hiding this comment.
When ConsoleApp mounts its normal live host with FIXTURE_TENANT, the CanvasStore constructor immediately creates and commits the default canvas, invoking this callback before this.http is assigned later in the constructor. graphToObjectRefs always returns the canvas ref, so the first iteration calls this.http.emit on undefined and crashes the entire Console during hydration. Construct HttpBlockHost first or suppress persistence during initial canvas creation.
Useful? React with 👍 / 👎.
| query: { | ||
| types: [ | ||
| 'model-scope', |
There was a problem hiding this comment.
In a normally seeded workspace, this query returns no model-scope: a repo-wide search finds no object creation or projection for that type, while modelScopeFromSet requires one with a nonempty topic_id. Consequently ModelView keeps topicId empty, never fetches the observed model, and disables every pin or proposal action. Seed or derive a concrete topic-scoped object as part of this view instance rather than querying for a type that has no producer.
AGENTS.md reference: apps/console/AGENTS.md:L132-L137
Useful? React with 👍 / 👎.
| function queryPrograms(host: BlockHost): ObjectSet { | ||
| const result = host.query(PROGRAM_QUERY); | ||
| if (result instanceof Promise) return emptySet(); |
There was a problem hiding this comment.
For the live ConsoleBlockHost, program types fall through to HttpBlockHost.query, which always returns a Promise. This branch replaces every such result with an empty set whose subscription is a no-op, and neither createStore nor subscribe ever awaits the original request, so the new Program surface permanently displays “No program loaded” even when the backend returned nodes. Resolve the async query into store state and attach its returned subscription.
Useful? React with 👍 / 👎.
| .json(&json!({ | ||
| "jsonrpc": "2.0", | ||
| "id": "commonplace-reconstruction", | ||
| "method": "tools/call", |
There was a problem hiding this comment.
Against the configured Theorem /mcp endpoint, this sends tools/call as the first and only request. That endpoint requires the initialize negotiation, Mcp-Session-Id propagation, notifications/initialized, and then the correlated tool request; it may also return an SSE response rather than the JSON body parsed below. As written, both reconstructionRun and reconstruct are rejected before structuredContent can be returned, so reuse the repository's full MCP lifecycle transport or implement that handshake here.
Useful? React with 👍 / 👎.
| await host.emit({ | ||
| kind: 'create', | ||
| type, |
There was a problem hiding this comment.
After any successful pin or unpin, these create and preceding delete actions are sent to ConsoleBlockHost, but that host returns an accepted receipt for non-layout creates and for deletes whose IDs are not layout nodes without forwarding either action to HttpBlockHost. The synchronization therefore reports success while the shared object graph remains absent or stale, so other panes and subsequent object queries cannot observe the declared metadata. Add live wire-through for these domain mutations or use the backend object seam directly.
AGENTS.md reference: apps/console/AGENTS.md:L6-L9
Useful? React with 👍 / 👎.
| void host.emit({ | ||
| kind: 'link', | ||
| from: connection.source!, | ||
| edge: 'DEPENDS_ON', | ||
| to: connection.target!, |
There was a problem hiding this comment.
When a user connects two Goal Stack tasks, this emits a generic link action, but ConsoleBlockHost.emit explicitly treats unowned links as UI-only and immediately returns accepted without updating the plan or calling the Harness. Because the graph is rebuilt from the polled plan snapshot, the new edge disappears and is never persisted while the UI clears its error as though the operation succeeded. Send an authoritative dependency action through the existing /api/harness/plan mutation path and handle its receipt.
Useful? React with 👍 / 👎.
| const expectedNavIds = new Set( | ||
| declared.objectTypes.map((item) => navObjectId(item.key || item.id)), | ||
| ); | ||
| const currentNav = await host.query({ types: [NAV_ITEM_TYPE] }); | ||
| for (const object of currentNav.objects) { | ||
| const isObjectKind = object.properties.item_kind === 'object'; | ||
| const isWorkspace = object.properties.scope_kind !== 'user'; | ||
| if (isObjectKind && isWorkspace && !expectedNavIds.has(object.id)) { |
There was a problem hiding this comment.
When a tenant has declared models for multiple topics, this unfiltered query loads every NavItem, but expectedNavIds contains only the object types from the topic currently being synchronized. The loop consequently deletes and globally retires workspace navigation entries created for other topics, even though newly created entries carry topic_id. Filter currentNav by that topic_id or reconcile against the union of declared topics before retiring entries.
Useful? React with 👍 / 👎.
| const handleCopyPath = React.useCallback(() => { | ||
| navigator.clipboard.writeText(path).then(() => { | ||
| setPathCopied(true) | ||
| setTimeout(() => setPathCopied(false), 1500) |
There was a problem hiding this comment.
In the newly reachable JSON viewer, copying a path calls navigator.clipboard directly, and the same bypass appears in the added JSON, diff, and log copy controls. On insecure origins, locked-down embeds, or denied clipboard permission, this access can be absent or reject without transitioning the UI to an unavailable state; it also duplicates timers and error handling that useCopyToClipboard already centralizes. Replace these direct calls with the repository copy seam.
AGENTS.md reference: apps/console/AGENTS.md:L118-L118
Useful? React with 👍 / 👎.
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
Travis-Gilbert
changed the title
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/commonplace-api/src/find.rs (1)
201-212: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy liftEnforce
node_limitbefore materializing the snapshot.
graph_snapshot()returns the full node and edge sets, and the limit is applied only afterward. A large graph can therefore allocate an unbounded snapshot despiteFindConfig::node_limit. Add a bounded snapshot API, or query bounded nodes first and fetch only edges between those nodes.🤖 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/commonplace-api/src/find.rs` around lines 201 - 212, Update the graph-loading logic around graph_snapshot and the subsequent nodes collection so FindConfig::node_limit is enforced before materializing unbounded snapshot data. Use an existing bounded snapshot API if available; otherwise query at most config.node_limit nodes first and retrieve only edges connecting those nodes, while preserving the current fallback behavior and NodeRecord output.
🧹 Nitpick comments (4)
apps/console/package.json (1)
28-29: 📐 Maintainability & Code Quality | 🔵 TrivialVerify the Playwright visual baseline is mandatory in CI.
gatesdoes not invoketest:e2e; ensure a required CI job runs the visual baseline before merge.As per coding guidelines, “All required gates must pass before merge: fence, register, contrast, motion, icons, canonical-root, and Playwright visual baseline.”
#!/bin/bash fd -a 'playwright.config.*' apps/console . rg -n -C2 'test:e2e|playwright test|toHaveScreenshot|screenshot' .github apps/console🤖 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 28 - 29, Make the Playwright visual baseline mandatory before merge by adding test:e2e (or an equivalent Playwright test command) to the required CI gate workflow, rather than only defining it in package scripts. Locate the existing CI gate configuration and ensure the job runs apps/console’s Playwright tests and is included among required checks; preserve the existing gates unchanged.apps/console/src/components/json-viewer.tsx (2)
9-11: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStale sourcing header.
The header claims themes are "powered by shiki" and lists
lucide-reactas the only dependency, but this file imports themes from@/styles/jalco-json-themesand has no shiki dependency. Also, unlikekbd.tsxandstatus-indicator.tsx, this file carries noSPEC-CONSOLE-COMPONENT-SOURCING-1.0sourcing marker, which the newlint-sourcing.mjsgate in this PR may expect.Also applies to: 21-21
🤖 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/json-viewer.tsx` around lines 9 - 11, Update the json-viewer.tsx header to remove the inaccurate Shiki-powered theme claim and correct the dependency description to match the file’s actual imports, including `@/styles/jalco-json-themes`. Add the SPEC-CONSOLE-COMPONENT-SOURCING-1.0 sourcing marker in the same manner as kbd.tsx and status-indicator.tsx so the lint-sourcing gate recognizes the component.
351-355: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDead branch:
filteredEntriesis computed butdisplayEntriesnever uses it differently.
showAllis!searchQuery, sodisplayEntriesisentriesexactly whenfilteredEntrieswas not filtered, andfilteredEntrieswhen it was. The intermediateshowAllindirection makes it read as if unfiltered display is possible during search. Collapse to a single expression.♻️ Simplification
- const filteredEntries = searchQuery - ? entries.filter(([k, v]) => hasSearchMatch(v, k, searchQuery)) - : entries - const showAll = !searchQuery - const displayEntries = showAll ? entries : filteredEntries + const displayEntries = searchQuery + ? entries.filter(([k, v]) => hasSearchMatch(v, k, searchQuery)) + : entries🤖 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/json-viewer.tsx` around lines 351 - 355, In the display-entry selection near filteredEntries, remove the redundant showAll variable and collapse the logic into a single expression that uses entries when searchQuery is empty and filteredEntries otherwise. Preserve the existing hasSearchMatch filtering behavior.apps/console/src/components/diff-viewer.tsx (1)
197-203: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnreachable
elsebranch.
DiffLine["type"]is a closed union ofcontext,removed, andadded, all handled above; the trailingelse { i++ }is dead code.🤖 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/diff-viewer.tsx` around lines 197 - 203, Remove the unreachable trailing else branch from the diff-line processing logic, leaving the existing handling for context, removed, and added line types unchanged. Update the loop around the visible line-type checks so index advancement remains correct for each supported DiffLine type.
🤖 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 @.codex/config.toml:
- Around line 12-22: Remove any repeated table headers for the GitHub MCP tools
in the configuration, retaining exactly one declaration each for
get_pull_request, get_pull_request_status, get_pull_request_reviews, and
get_pull_request_comments with their existing approval_mode settings, so the
TOML parses successfully.
In `@apps/commonplace-api/src/reconstruction.rs`:
- Around line 70-73: Update the endpoint validation in the reconstruction
configuration flow to accept only https:// URLs before execute sends the Bearer
token. If local HTTP MCP support is required, gate it behind an explicit
development-only override; otherwise reject http:// with the existing
MCP_URL_ENV validation error.
In `@apps/console/scripts/check-register-lint.mjs`:
- Around line 33-50: Remove the registry-install exemption implemented by
isRegistryInstallPath and its callers, so components/ui and the listed registry
paths remain subject to arbitrary-value linting. Update the affected upstream
registry classes to use the pinned Int UI utility equivalents instead of
bypassing the gate.
In `@apps/console/scripts/lint-sourcing.mjs`:
- Around line 32-34: Replace the declaration-specific regex used by the
descriptorBlocks parsing flow with TypeScript AST traversal that identifies
every variable declaration whose type or satisfies expression is ViewDescriptor,
regardless of export status, variable naming, or closing syntax. Preserve
extraction of each descriptor initializer so all valid registry entries are
included in the sourcing gate.
In `@apps/console/src/components/command-menu-01.tsx`:
- Around line 65-66: Update the initial state in CommandMenu01 so the command
menu starts closed on mount, while preserving the existing open-state and
trigger behavior.
In `@apps/console/src/components/diff-viewer-client.tsx`:
- Around line 14-18: Replace all local clipboard implementations with the shared
useCopy hook. In apps/console/src/components/diff-viewer-client.tsx lines 14-18,
remove local copied state and navigator.clipboard.writeText usage; in
apps/console/src/components/json-viewer.tsx lines 218-223 and 575-582, update
handleCopyPath and copyJson to use the hook; in
apps/console/src/components/log-viewer.tsx lines 139-153, delete the local
useCopy implementation and import the shared hook, preserving centralized
rejection handling and copied-state behavior.
In `@apps/console/src/components/diff-viewer.tsx`:
- Around line 40-43: Update the patch handling in DiffViewer to preserve hunks
from every file returned by parsePatch instead of selecting parsed[0]. Flatten
the parsed entries’ hunks into the existing hunks collection, while keeping the
current empty-result behavior when no hunks are present.
- Line 290: Update the fullCode construction in the diff viewer so copied
content excludes removed lines and contains only context and added lines,
preserving their order. Ensure the Copy action uses this new-side content rather
than interleaving old and new lines.
- Around line 62-85: Skip jsdiff “No newline at end of file” marker lines in the
hunk-processing loop before the added, removed, or context branches. Update the
loop in the diff viewer so these backslash marker lines are not rendered and do
not increment oldNum or newNum, while preserving existing handling for actual
diff lines.
In `@apps/console/src/components/file-tree.tsx`:
- Around line 181-194: The tree node buttons in the file-tree component all use
tabIndex={0}, creating a tab stop for every node. Update the treeitem tabIndex
logic so exactly one node is tabbable and all others use -1, reusing the
existing active/focused-node state and arrow-key navigation behavior in the
surrounding component.
- Around line 35-41: Synchronize the expanded state with changes to
initialExpanded in the FileTree component: add an effect that updates
setExpanded whenever defaultExpanded or tree causes initialExpanded to change.
Preserve the existing initialExpanded calculation and expansion behavior for
unchanged props.
In `@apps/console/src/components/ground/MaterialLayer.tsx`:
- Line 239: Update the mutation observer configuration in the material-layer
component to observe the data-block-size attribute alongside the existing
filters, so changes invalidate and repaint the affected island using the
recalculated nodeRadius.
In `@apps/console/src/components/json-viewer.tsx`:
- Around line 128-137: Complete the --ij-* register reskin across all listed
sites, removing raw palette utilities, arbitrary Tailwind values, and upstream
shadcn styling. In apps/console/src/components/json-viewer.tsx lines 128-137,
update fallbackMap and container chrome; in apps/console/src/components/kbd.tsx
lines 14-29, replace arbitrary border and text sizes; in
apps/console/src/components/log-viewer.tsx lines 66-92, reskin verbose entries
and replace arbitrary width, margin, and scrollbar utilities. In
apps/console/src/components/diff-viewer.tsx lines 100-118,
apps/console/src/components/file-tree.tsx lines 183-190,
apps/console/src/components/ui/alert.tsx lines 6-20, and
apps/console/src/components/ui/calendar.tsx lines 35-40, replace the cited
palette, opacity, sizing, grid, cell-metric, and background classes with
established IntelliJ register tokens and scale utilities.
In `@apps/console/src/components/kbd.tsx`:
- Around line 14-29: Replace the arbitrary-value classes in the kbd variant
map—border-b-[2px], border-b-[3px], text-[10px], and text-[11px]—with the
corresponding approved register token or scale utility classes, while preserving
the existing visual sizes and border weights for the raised, sculpted, sm, and
md variants.
In `@apps/console/src/components/log-viewer.tsx`:
- Around line 13-18: Replace all em dashes in the LogViewer documentation
comments, including the export descriptions and colorScale comments near the
referenced locations, with standard punctuation such as colons, commas, or
periods; do not alter the documented meaning.
- Around line 193-199: Update the download flow around the anchor created in the
log export handler so the object URL remains valid long enough for the browser
to start the download: defer URL.revokeObjectURL(url) rather than calling it
synchronously after a.click(), and remove the temporary anchor after triggering
the download.
In `@apps/console/src/components/material/ShaderSurface.tsx`:
- Around line 433-435: Update the theme MutationObserver setup in ShaderSurface
so it also observes the data-register attribute alongside data-theme and class.
Ensure register changes rerun the existing resolveRuntime(...).uniforms refresh
for mounted shaders while preserving the pinned Int UI light/dark registers and
gated two-knob theme behavior.
In `@apps/console/src/components/status-indicator.tsx`:
- Around line 107-109: Update the animated span in the status indicator to
disable the perpetual ping under reduced-motion preferences using the project’s
motion-reduce utility, leaving it settled and static while preserving normal
animation otherwise. Add this animation to the interaction inventory using the
established entry format, and ensure the animation remains limited to transform
and opacity.
In `@apps/console/src/components/ui/checkbox.tsx`:
- Line 17: Apply the reduced-motion contract across all listed sites: in
apps/console/src/components/ui/checkbox.tsx:17 remove transition-shadow or
replace it with approved transform/opacity motion plus a static reduced-motion
state; in apps/console/src/components/ui/dropdown-menu.tsx:45 and :233 add
static reduced-motion behavior and register the menu and submenu interactions;
in apps/console/src/components/ui/input.tsx:11-13 remove color and box-shadow
transitions so focus settles statically; in
apps/console/src/components/ui/sheet.tsx:39 add a static reduced-motion overlay
and register the interaction, and at :63-71 restrict motion to transform/opacity
with a static reduced-motion state; in
apps/console/src/components/ui/skeleton.tsx:7 disable pulse under reduced motion
and register the loading interaction.
---
Outside diff comments:
In `@apps/commonplace-api/src/find.rs`:
- Around line 201-212: Update the graph-loading logic around graph_snapshot and
the subsequent nodes collection so FindConfig::node_limit is enforced before
materializing unbounded snapshot data. Use an existing bounded snapshot API if
available; otherwise query at most config.node_limit nodes first and retrieve
only edges connecting those nodes, while preserving the current fallback
behavior and NodeRecord output.
---
Nitpick comments:
In `@apps/console/package.json`:
- Around line 28-29: Make the Playwright visual baseline mandatory before merge
by adding test:e2e (or an equivalent Playwright test command) to the required CI
gate workflow, rather than only defining it in package scripts. Locate the
existing CI gate configuration and ensure the job runs apps/console’s Playwright
tests and is included among required checks; preserve the existing gates
unchanged.
In `@apps/console/src/components/diff-viewer.tsx`:
- Around line 197-203: Remove the unreachable trailing else branch from the
diff-line processing logic, leaving the existing handling for context, removed,
and added line types unchanged. Update the loop around the visible line-type
checks so index advancement remains correct for each supported DiffLine type.
In `@apps/console/src/components/json-viewer.tsx`:
- Around line 9-11: Update the json-viewer.tsx header to remove the inaccurate
Shiki-powered theme claim and correct the dependency description to match the
file’s actual imports, including `@/styles/jalco-json-themes`. Add the
SPEC-CONSOLE-COMPONENT-SOURCING-1.0 sourcing marker in the same manner as
kbd.tsx and status-indicator.tsx so the lint-sourcing gate recognizes the
component.
- Around line 351-355: In the display-entry selection near filteredEntries,
remove the redundant showAll variable and collapse the logic into a single
expression that uses entries when searchQuery is empty and filteredEntries
otherwise. Preserve the existing hasSearchMatch filtering behavior.
🪄 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: 614d6e0b-a517-4ae6-8ce9-ed96d8e1caec
⛔ Files ignored due to path filters (4)
apps/commonplace-api/Cargo.lockis excluded by!**/*.lockapps/console/docs/plans/isometric-register/canvas-three-connected-nodes.pngis excluded by!**/*.pngapps/console/e2e/appearance.spec.ts-snapshots/workspace-1280-light-darwin.pngis excluded by!**/*.pngapps/console/e2e/appearance.spec.ts-snapshots/workspace-1440-light-darwin.pngis excluded by!**/*.png
📒 Files selected for processing (74)
.codex/config.toml.theorem/learnings/019f5c09-7043-71c1-937e-1ab6e754ab8c.head.theorem/learnings/019f94da-53f0-73f2-a421-8eca16d7ae59.captured.theorem/learnings/019f94da-53f0-73f2-a421-8eca16d7ae59.head.theorem/learnings/019f9c4b-1c67-7b23-8b34-3806bc7efec3.head.theorem/learnings/019f9ca0-ad47-7300-b6d5-aaeb7f6e6ff6.head.theorem/learnings/019f9ffe-2161-7240-af78-22a8137a9a25.head.theorem/learnings/019f9ffe-b85e-7893-b51c-4ae5dab30d99.head.theorem/learnings/019f9fff-4295-74d0-841f-7c960cd4c348.head.theorem/learnings/019fa03e-81c7-76e3-a025-9a607a5c4b93.head.theorem/learnings/019fa190-3cbb-7d50-a5cc-e0c73eafa24f.head.theorem/learnings/019fa228-165b-7200-8f42-4fd6d8e142a0.head.theorem/learnings/019fa38e-0811-7772-bafe-5fb66427ed17.head.theorem/learnings/019fa450-a68e-7af2-95ce-a8f990fbf75f.head.theorem/learnings/5c0cdd64-94fd-4234-a130-714757eb6ff8.headapps/commonplace-api/.env.exampleapps/commonplace-api/src/find.rsapps/commonplace-api/src/lib.rsapps/commonplace-api/src/organize.rsapps/commonplace-api/src/publish.rsapps/commonplace-api/src/reconstruction.rsapps/commonplace-api/src/salience.rsapps/commonplace-api/src/save_url.rsapps/commonplace-api/src/schema/find.rsapps/commonplace-api/src/schema/mod.rsapps/commonplace-api/src/serve.rsapps/commonplace-api/tests/organize_acceptance.rsapps/console/docs/plans/consolidation/sourcing-audit.mdapps/console/e2e/console-sidebar.spec.tsapps/console/package.jsonapps/console/scripts/check-register-lint.mjsapps/console/scripts/lint-sourcing.mjsapps/console/src/app/api/agent-address/aliases/route.tsapps/console/src/app/program/page.tsxapps/console/src/components/blocks/BlockShell.test.tsxapps/console/src/components/chat/RuntimeComposer.tsxapps/console/src/components/chat/toolMeta.tsapps/console/src/components/command-menu-01.tsxapps/console/src/components/context/LayerPicker.tsxapps/console/src/components/diff-viewer-client.tsxapps/console/src/components/diff-viewer.tsxapps/console/src/components/file-tree.tsxapps/console/src/components/ground/MaterialLayer.tsxapps/console/src/components/jalco/commit-graph.tsxapps/console/src/components/jalco/diff-viewer.tsxapps/console/src/components/jalco/file-tree.tsxapps/console/src/components/jalco/index.tsapps/console/src/components/jalco/json-viewer.tsxapps/console/src/components/jalco/kbd.tsxapps/console/src/components/jalco/log-viewer.tsxapps/console/src/components/jalco/status-indicator.tsxapps/console/src/components/json-viewer.tsxapps/console/src/components/kbd.tsxapps/console/src/components/log-viewer.tsxapps/console/src/components/material/ShaderSurface.tsxapps/console/src/components/shell/IntuiShell.tsxapps/console/src/components/shell/Sidebar.tsxapps/console/src/components/sources/command-menu.tsxapps/console/src/components/sources/index.tsapps/console/src/components/sources/linear-combobox.tsxapps/console/src/components/status-indicator.tsxapps/console/src/components/ui/alert.tsxapps/console/src/components/ui/calendar.tsxapps/console/src/components/ui/checkbox.tsxapps/console/src/components/ui/command.tsxapps/console/src/components/ui/dropdown-menu.tsxapps/console/src/components/ui/input.tsxapps/console/src/components/ui/kbd.tsxapps/console/src/components/ui/separator.tsxapps/console/src/components/ui/sheet.tsxapps/console/src/components/ui/skeleton.tsxapps/console/src/components/ui/table.tsxapps/console/src/lib/block-placement.test.tsapps/console/src/lib/canvas/store.test.ts
💤 Files with no reviewable changes (6)
- apps/console/src/components/jalco/file-tree.tsx
- apps/console/src/components/jalco/kbd.tsx
- apps/console/src/components/jalco/log-viewer.tsx
- apps/console/src/components/jalco/diff-viewer.tsx
- apps/console/src/components/jalco/json-viewer.tsx
- apps/console/src/components/jalco/status-indicator.tsx
| [mcp_servers.github.tools.get_pull_request] | ||
| approval_mode = "approve" | ||
|
|
||
| [mcp_servers.github.tools.get_pull_request_status] | ||
| approval_mode = "approve" | ||
|
|
||
| [mcp_servers.github.tools.get_pull_request_reviews] | ||
| approval_mode = "approve" | ||
|
|
||
| [mcp_servers.github.tools.get_pull_request_comments] | ||
| approval_mode = "approve" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Remove duplicate MCP table declarations.
The supplied snippet repeats each GitHub MCP table header. If this duplication exists in .codex/config.toml, the TOML parser will reject the configuration. Keep one declaration per tool.
python - <<'PY'
from pathlib import Path
import tomllib
path = Path(".codex/config.toml")
try:
tomllib.loads(path.read_text())
except tomllib.TOMLDecodeError as error:
print(error)
raise SystemExit(1)
PY🤖 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 @.codex/config.toml around lines 12 - 22, Remove any repeated table headers
for the GitHub MCP tools in the configuration, retaining exactly one declaration
each for get_pull_request, get_pull_request_status, get_pull_request_reviews,
and get_pull_request_comments with their existing approval_mode settings, so the
TOML parses successfully.
| if endpoint.starts_with("http://") || endpoint.starts_with("https://") { | ||
| Ok(endpoint) | ||
| } else { | ||
| Err(format!("{MCP_URL_ENV} must be an HTTP(S) URL")) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Require HTTPS for the MCP endpoint.
This accepts http://, while execute sends the reconstruction token as a Bearer credential. A plaintext endpoint exposes that credential. Require https://, with an explicit development-only override if local MCP support is necessary.
Proposed fix
- if endpoint.starts_with("http://") || endpoint.starts_with("https://") {
+ if endpoint.starts_with("https://") {
Ok(endpoint)
} else {
- Err(format!("{MCP_URL_ENV} must be an HTTP(S) URL"))
+ Err(format!("{MCP_URL_ENV} must be an HTTPS URL"))
}📝 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.
| if endpoint.starts_with("http://") || endpoint.starts_with("https://") { | |
| Ok(endpoint) | |
| } else { | |
| Err(format!("{MCP_URL_ENV} must be an HTTP(S) URL")) | |
| if endpoint.starts_with("https://") { | |
| Ok(endpoint) | |
| } else { | |
| Err(format!("{MCP_URL_ENV} must be an HTTPS URL")) |
🤖 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/commonplace-api/src/reconstruction.rs` around lines 70 - 73, Update the
endpoint validation in the reconstruction configuration flow to accept only
https:// URLs before execute sends the Bearer token. If local HTTP MCP support
is required, gate it behind an explicit development-only override; otherwise
reject http:// with the existing MCP_URL_ENV validation error.
| // Registry installs (SC1) land with upstream metric arbitrary values | ||
| // (ring-[3px], text-[10px]). Color must still clear: hex, raw color | ||
| // functions, and Tailwind palette utilities remain banned everywhere. | ||
| function isRegistryInstallPath(file) { | ||
| const rel = path.relative(srcRoot, file).split(path.sep).join('/'); | ||
| return ( | ||
| rel.startsWith('components/ui/') || | ||
| rel.startsWith('components/data-table/') || | ||
| rel === 'components/json-viewer.tsx' || | ||
| rel === 'components/log-viewer.tsx' || | ||
| rel === 'components/diff-viewer.tsx' || | ||
| rel === 'components/diff-viewer-client.tsx' || | ||
| rel === 'components/file-tree.tsx' || | ||
| rel === 'components/command-menu-01.tsx' || | ||
| rel === 'components/status-indicator.tsx' || | ||
| rel === 'components/kbd.tsx' || | ||
| rel === 'components/calendar-date-picker.tsx' | ||
| ); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Do not exempt registry installs from the arbitrary-value rule.
This bypass lets current and future files under components/ui/ and the listed paths use prohibited arbitrary Tailwind classes. Adapt upstream registry metrics to the pinned Int UI utilities instead of disabling the gate.
Proposed fix
- const registryInstall = isRegistryInstallPath(file);
lines.forEach((line, index) => {
for (const [name, re] of [
['hex literal', HEX_RE],
['raw color function', RAW_COLOR_FN_RE],
- ...(registryInstall ? [] : [['arbitrary-value class', ARBITRARY_RE]]),
+ ['arbitrary-value class', ARBITRARY_RE],As per coding guidelines, “Do not use … arbitrary-value Tailwind classes.”
Also applies to: 92-97
🤖 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/scripts/check-register-lint.mjs` around lines 33 - 50, Remove
the registry-install exemption implemented by isRegistryInstallPath and its
callers, so components/ui and the listed registry paths remain subject to
arbitrary-value linting. Update the affected upstream registry classes to use
the pinned Int UI utility equivalents instead of bypassing the gate.
Source: Coding guidelines
| const registryText = readFileSync(registryPath, 'utf8'); | ||
| const descriptorBlocks = [...registryText.matchAll(/const\s+[A-Z0-9_]+\s*:\s*ViewDescriptor\s*=\s*\{([\s\S]*?)\n\};/g)]; | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Parse descriptors with a TypeScript AST, not this declaration-specific regex.
Valid declarations using export, non-uppercase names, satisfies ViewDescriptor, or a different closing format are silently omitted when at least one block matches. That lets descriptors bypass the sourcing gate.
#!/bin/bash
ast-grep outline apps/console/src/views/registry.tsx --items all
rg -n -C2 'ViewDescriptor|satisfies\s+ViewDescriptor|export\s+const' \
apps/console/src/views/registry.tsx🤖 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/scripts/lint-sourcing.mjs` around lines 32 - 34, Replace the
declaration-specific regex used by the descriptorBlocks parsing flow with
TypeScript AST traversal that identifies every variable declaration whose type
or satisfies expression is ViewDescriptor, regardless of export status, variable
naming, or closing syntax. Preserve extraction of each descriptor initializer so
all valid registry entries are included in the sourcing gate.
| export function CommandMenu01() { | ||
| const [open, setOpen] = useState(true); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Start the command menu closed.
The dialog opens immediately on mount, so the trigger only becomes useful after dismissal.
Proposed fix
- const [open, setOpen] = useState(true);
+ const [open, setOpen] = useState(false);📝 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.
| export function CommandMenu01() { | |
| const [open, setOpen] = useState(true); | |
| export function CommandMenu01() { | |
| const [open, setOpen] = useState(false); |
🤖 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/command-menu-01.tsx` around lines 65 - 66, Update
the initial state in CommandMenu01 so the command menu starts closed on mount,
while preserving the existing open-state and trigger behavior.
| * Exports: | ||
| * - LogViewerTerminal — full CLI-style interface with toolbar, line numbers, and timestamps | ||
| * - LogViewerMinimal — simple scrolling log lines for compact contexts | ||
| * - LogViewerFilterable — includes level filtering (info/warn/error/debug) | ||
| * | ||
| * Dependencies: lucide-react |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Em dashes in comments and doc strings.
Lines 14 to 16 ("LogViewerTerminal — full CLI-style interface...") and the three colorScale doc comments ("Merges with defaults — only override what you need.") carry em dashes copied from upstream. Replace with a colon, comma, or period.
As per coding guidelines, "Do not use em dashes or en dashes in code comments, UI strings, or markdown. Use punctuation such as colons, periods, commas, semicolons, or parentheses instead."
Also applies to: 264-264, 469-469, 564-564
🤖 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/log-viewer.tsx` around lines 13 - 18, Replace all
em dashes in the LogViewer documentation comments, including the export
descriptions and colorScale comments near the referenced locations, with
standard punctuation such as colons, commas, or periods; do not alter the
documented meaning.
Source: Coding guidelines
| const blob = new Blob([text], { type: "text/plain" }) | ||
| const url = URL.createObjectURL(blob) | ||
| const a = document.createElement("a") | ||
| a.href = url | ||
| a.download = `logs-${new Date().toISOString().slice(0, 19).replace(/:/g, "-")}.txt` | ||
| a.click() | ||
| URL.revokeObjectURL(url) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Revoke the object URL after the download starts, and clean up the anchor.
URL.revokeObjectURL(url) runs synchronously on the line after a.click(), which can invalidate the blob before the browser has begun fetching it (notably Firefox). Defer the revoke.
🐛 Proposed fix
const a = document.createElement("a")
a.href = url
a.download = `logs-${new Date().toISOString().slice(0, 19).replace(/:/g, "-")}.txt`
+ a.style.display = "none"
+ document.body.appendChild(a)
a.click()
- URL.revokeObjectURL(url)
+ a.remove()
+ window.setTimeout(() => URL.revokeObjectURL(url), 0)📝 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 blob = new Blob([text], { type: "text/plain" }) | |
| const url = URL.createObjectURL(blob) | |
| const a = document.createElement("a") | |
| a.href = url | |
| a.download = `logs-${new Date().toISOString().slice(0, 19).replace(/:/g, "-")}.txt` | |
| a.click() | |
| URL.revokeObjectURL(url) | |
| const blob = new Blob([text], { type: "text/plain" }) | |
| const url = URL.createObjectURL(blob) | |
| const a = document.createElement("a") | |
| a.href = url | |
| a.download = `logs-${new Date().toISOString().slice(0, 19).replace(/:/g, "-")}.txt` | |
| a.style.display = "none" | |
| document.body.appendChild(a) | |
| a.click() | |
| a.remove() | |
| window.setTimeout(() => URL.revokeObjectURL(url), 0) |
🤖 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/log-viewer.tsx` around lines 193 - 199, Update
the download flow around the anchor created in the log export handler so the
object URL remains valid long enough for the browser to start the download:
defer URL.revokeObjectURL(url) rather than calling it synchronously after
a.click(), and remove the temporary anchor after triggering the download.
| mount.setUniforms(resolveRuntime(shader, runtimeRef.current, reduced).uniforms); | ||
| }); | ||
| observer.observe(document.documentElement, { attributes: true, attributeFilter: ['data-theme', 'class'] }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Refresh shader uniforms when the Int UI register changes.
The observer omits data-register, so mounted shaders retain stale resolved RGBA colors after a register switch that changes --ij-* tokens without changing data-theme or class.
Proposed fix
- observer.observe(document.documentElement, { attributes: true, attributeFilter: ['data-theme', 'class'] });
+ observer.observe(document.documentElement, {
+ attributes: true,
+ attributeFilter: ['data-theme', 'data-register', 'class'],
+ });As per coding guidelines, use the pinned Int UI light/dark registers and gated two-knob theme engine.
📝 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.
| mount.setUniforms(resolveRuntime(shader, runtimeRef.current, reduced).uniforms); | |
| }); | |
| observer.observe(document.documentElement, { attributes: true, attributeFilter: ['data-theme', 'class'] }); | |
| mount.setUniforms(resolveRuntime(shader, runtimeRef.current, reduced).uniforms); | |
| }); | |
| observer.observe(document.documentElement, { | |
| attributes: true, | |
| attributeFilter: ['data-theme', 'data-register', 'class'], | |
| }); |
🤖 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/material/ShaderSurface.tsx` around lines 433 -
435, Update the theme MutationObserver setup in ShaderSurface so it also
observes the data-register attribute alongside data-theme and class. Ensure
register changes rerun the existing resolveRuntime(...).uniforms refresh for
mounted shaders while preserving the pinned Int UI light/dark registers and
gated two-knob theme behavior.
Source: Coding guidelines
| <span | ||
| className={cn('absolute inset-0 rounded-full animate-ping opacity-40', config.dot)} | ||
| /> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Perpetual animate-ping needs a reduced-motion settled state and an inventory entry.
This dot animates continuously with no motion-reduce guard, so it keeps pulsing when the user has requested reduced motion, and it is a new animation that the motion gate expects to find in the interaction inventory.
As per coding guidelines, "Every animation must appear in the interaction inventory, use transform and opacity only, and render settled and static when reduced motion is enabled."
🛠️ Reduced-motion guard
<span
- className={cn('absolute inset-0 rounded-full animate-ping opacity-40', config.dot)}
+ className={cn(
+ 'absolute inset-0 rounded-full animate-ping opacity-40 motion-reduce:animate-none',
+ config.dot,
+ )}
/>📝 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.
| <span | |
| className={cn('absolute inset-0 rounded-full animate-ping opacity-40', config.dot)} | |
| /> | |
| <span | |
| className={cn( | |
| 'absolute inset-0 rounded-full animate-ping opacity-40 motion-reduce:animate-none', | |
| config.dot, | |
| )} | |
| /> |
🤖 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/status-indicator.tsx` around lines 107 - 109,
Update the animated span in the status indicator to disable the perpetual ping
under reduced-motion preferences using the project’s motion-reduce utility,
leaving it settled and static while preserving normal animation otherwise. Add
this animation to the interaction inventory using the established entry format,
and ensure the animation remains limited to transform and opacity.
Source: Coding guidelines
| <CheckboxPrimitive.Root | ||
| data-slot="checkbox" | ||
| className={cn( | ||
| "peer size-4 shrink-0 rounded-[4px] border border-input shadow-xs transition-shadow outline-none focus-visible:border-ring focus-visible:ring-[3px] focus-visible:ring-ring/50 disabled:cursor-not-allowed disabled:opacity-50 aria-invalid:border-destructive aria-invalid:ring-destructive/20 data-[state=checked]:border-primary data-[state=checked]:bg-primary data-[state=checked]:text-primary-foreground dark:bg-input/30 dark:aria-invalid:ring-destructive/40 dark:data-[state=checked]:bg-primary", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Implement the console reduced-motion contract for every new interaction.
These components animate without a static reduced-motion branch. Additionally, transition-shadow, transition-[color,box-shadow], and unrestricted transition animate properties outside transform and opacity.
apps/console/src/components/ui/checkbox.tsx#L17-L17: remove the shadow transition or replace it with an approved motion treatment and a reduced-motion static state.apps/console/src/components/ui/dropdown-menu.tsx#L45-L45: add reduced-motion static behavior and register the menu interaction.apps/console/src/components/ui/dropdown-menu.tsx#L233-L233: add reduced-motion static behavior and register the submenu interaction.apps/console/src/components/ui/input.tsx#L11-L13: remove color and box-shadow transitions; focus styling should settle statically under reduced motion.apps/console/src/components/ui/sheet.tsx#L39-L39: add a static reduced-motion overlay state and register the interaction.apps/console/src/components/ui/sheet.tsx#L63-L71: constrain motion to transform and opacity, then add a reduced-motion static state.apps/console/src/components/ui/skeleton.tsx#L7-L7: disable the pulse under reduced motion and register the loading interaction.
As per coding guidelines: "Every animation must appear in the interaction inventory, use transform and opacity only, and render settled/static under reduced motion."
📍 Affects 5 files
apps/console/src/components/ui/checkbox.tsx#L17-L17(this comment)apps/console/src/components/ui/dropdown-menu.tsx#L45-L45apps/console/src/components/ui/dropdown-menu.tsx#L233-L233apps/console/src/components/ui/input.tsx#L11-L13apps/console/src/components/ui/sheet.tsx#L39-L39apps/console/src/components/ui/sheet.tsx#L63-L71apps/console/src/components/ui/skeleton.tsx#L7-L7
🤖 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/ui/checkbox.tsx` at line 17, Apply the
reduced-motion contract across all listed sites: in
apps/console/src/components/ui/checkbox.tsx:17 remove transition-shadow or
replace it with approved transform/opacity motion plus a static reduced-motion
state; in apps/console/src/components/ui/dropdown-menu.tsx:45 and :233 add
static reduced-motion behavior and register the menu and submenu interactions;
in apps/console/src/components/ui/input.tsx:11-13 remove color and box-shadow
transitions so focus settles statically; in
apps/console/src/components/ui/sheet.tsx:39 add a static reduced-motion overlay
and register the interaction, and at :63-71 restrict motion to transform/opacity
with a static reduced-motion state; in
apps/console/src/components/ui/skeleton.tsx:7 disable pulse under reduced motion
and register the loading interaction.
Source: Coding guidelines
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c43611d3c
ℹ️ 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".
| layoutObject('threads.vi-list', 'view-instance', { | ||
| descriptor_id: 'chat.surface', title: 'Threads', | ||
| descriptor_id: 'thread.list', title: 'Threads', | ||
| query: { types: ['thread'] } as unknown as JsonValue, |
There was a problem hiding this comment.
Fresh evidence after the earlier Threads comment is that the replacement list still queries thread, while thread-persistence.ts writes every durable transcript as chat-thread. In a live console, ConsoleBlockHost treats thread as a local-only type and never queries the authenticated object seam, so users with persisted chats still see “No threads yet.” Query CHAT_THREAD_TYPE and update the list filter accordingly.
AGENTS.md reference: apps/console/AGENTS.md:L170-L171
Useful? React with 👍 / 👎.
| @@ -0,0 +1,17 @@ | |||
| import ReconstructionViewer from '@/components/commonplace/scene-host/ReconstructionViewer'; | |||
There was a problem hiding this comment.
A repo-wide file search shows that apps/web now contains only these four added files and has no package manifest, while the root build scripts and both Railway configurations build only apps/console. Consequently this route is never compiled or deployed, and its imports also refer to modules absent from the residual apps/web tree. Move the route and viewer into the Console descriptor/router path, or remove the unreachable implementation.
Useful? React with 👍 / 👎.
| const violations = []; | ||
|
|
||
| const registryText = readFileSync(registryPath, 'utf8'); | ||
| const descriptorBlocks = [...registryText.matchAll(/const\s+[A-Z0-9_]+\s*:\s*ViewDescriptor\s*=\s*\{([\s\S]*?)\n\};/g)]; |
There was a problem hiding this comment.
This regex matches only constants annotated exactly as ViewDescriptor, but the same registry contains nine ConsoleViewDescriptor constants, including RECORD_TABLE and MARKDOWN_DOC, which are also registered descriptors and currently omit the new sourcing field. Running the gate reports 23 descriptors although the registry contains 32, so those source declarations can regress while CI remains green; match both descriptor types or inspect the exported registry structurally.
AGENTS.md reference: apps/console/AGENTS.md:L87-L91
Useful? React with 👍 / 👎.
| | { | ||
| readonly op_range?: never; | ||
| /** Explicit marker for receipts minted before durable operation ranges. */ | ||
| readonly legacy_without_op_range: true; |
There was a problem hiding this comment.
For every action sent through HttpBlockHost, the Rust ObjectActionReceipt serializes neither op_range nor legacy_without_op_range, and the HTTP adapter simply casts that JSON to this newly exhaustive union. Live callers therefore receive values that satisfy neither branch and cannot reliably distinguish ledger-backed receipts from legacy ones. Add the legacy marker or operation range on the server, or normalize the response before exposing this type.
Useful? React with 👍 / 👎.
| ['hex literal', HEX_RE], | ||
| ['raw color function', RAW_COLOR_FN_RE], | ||
| ['arbitrary-value class', ARBITRARY_RE], | ||
| ...(registryInstall ? [] : [['arbitrary-value class', ARBITRARY_RE]]), |
There was a problem hiding this comment.
For every path classified as a registry install, this removes ARBITRARY_RE from the checks entirely. The newly reachable JSON viewer contains metrics such as text-[10px], so the gate now reports success while raw Tailwind values bypass the register, and future imported components receive the same blanket exemption. Keep the arbitrary-value check enabled and translate upstream metrics to named register tokens instead.
AGENTS.md reference: apps/console/AGENTS.md:L83-L85
Useful? React with 👍 / 👎.
| }; | ||
|
|
||
| export function getToolMeta(toolName: string): ToolMeta { | ||
| return REGISTRY[toolName] ?? DEFAULT_META; |
There was a problem hiding this comment.
Whenever the assistant emits a tool not present in this six-entry registry, getToolMeta returns the shared DEFAULT_META whose label is only Tool. ThreadView now uses that label for both the speaker and fallback summary, so unregistered calls become indistinguishable and the actual toolName disappears from the transcript. Build fallback metadata from toolName, or retain the name when rendering the excerpt.
Useful? React with 👍 / 👎.
| getComputedStyle(node).getPropertyValue('--ij-island-radius'), | ||
| fallbackRadius, | ||
| ); | ||
| const nodeRadius = radiusBySize[node.dataset.blockSize ?? ''] ?? fallbackRadius; |
There was a problem hiding this comment.
For ground blocks, data-block-size is set on the BlockCanvas wrapper while data-island is set on its child BlockShell, so node.dataset.blockSize is empty here. Every such island therefore uses the root fallback radius, while its DOM clip still inherits the size-specific 4/6/8/12/16px radius from the ancestor, producing visibly mismatched MaterialLayer SDF corners. Read the computed --ij-island-radius as before or resolve the nearest size-bearing ancestor.
Useful? React with 👍 / 👎.
| return false; | ||
| } | ||
|
|
||
| export function FindIndexView(_props: ViewRenderProps) { |
There was a problem hiding this comment.
A repo-wide search of this commit finds FindIndexView only in this file: registry.tsx neither imports it nor declares the documented find.index descriptor, although the sourcing audit says that unavailable state shipped. Because the shell mounts only CONSOLE_VIEW_DESCRIPTORS, this component is tree-shaken and users cannot reach even its designed backend-unavailable state. Add the descriptor registration and corresponding seeded view instance.
AGENTS.md reference: apps/console/AGENTS.md:L145-L150
Useful? React with 👍 / 👎.
| const declaredNodes: Node[] = declared.objectTypes.map((type, index) => { | ||
| const divergences = divergencesForType(declared, type.id); | ||
| const count = divergences.reduce((sum, item) => sum + item.count, 0); | ||
| const showBadge = type.enforcement === 'warn' && count > 0; |
There was a problem hiding this comment.
When a declared type has divergences under reject or observe, this condition suppresses all divergence indication; moreover, the added DivergenceBadge is never mounted anywhere. Even warn exposes only an aggregate count, leaving each divergence's fieldKey, kind, and signalNodeIds inaccessible. Render the divergence details independently of enforcement mode so rejected or observed schema violations are not hidden.
Useful? React with 👍 / 👎.
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 34344f4846
ℹ️ 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 missing = localObjects.filter((object) => !remoteIds.has(object.id)); | ||
| if (missing.length === 0) return [...remoteObjects]; |
There was a problem hiding this comment.
Fresh evidence after the earlier remote-layout finding is that this replacement migration considers only missing IDs and returns immediately when every node already exists. Seed upgrades in hydrateLayout also revise existing nodes, such as cards.region-editor.kind, containment, and stripe_order; none of those updates are emitted, and replaceLayout then overwrites the upgraded local objects with the stale remote versions. Reconcile changed properties and relations as well as creating missing nodes.
AGENTS.md reference: apps/console/AGENTS.md:L145-L150
Useful? React with 👍 / 👎.
| ) : ( | ||
| <pre className="overflow-x-auto whitespace-pre-wrap font-ij-mono text-ij-ink-info"> | ||
| {[props.argsText, resultText].filter(Boolean).join('\n\n') || meta.label} | ||
| </pre> |
There was a problem hiding this comment.
When a registered recall, remember, encode, or plan call returns an object, resultObject is populated but resultText is null and useJson is false because those tools are marked markdown or text. This fallback consequently renders only the arguments or label and silently drops the tool result that the previous implementation serialized. Render or stringify structured results regardless of the selected presentation.
Useful? React with 👍 / 👎.
| }, | ||
| }); | ||
| if (!updated.ok) return; | ||
| navigateTo('console-records', `/records?type=${encodeURIComponent(objectTypeId)}`); |
There was a problem hiding this comment.
Fresh evidence after the earlier registry-navigation finding is that the click path now patches the view instance before navigating, but the generated /records?type=... URL has no consumer: app/records/page.tsx only re-exports console-surface-page, and a repo-wide search finds no Records searchParams handling. A reload, shared link, or browser Back navigation therefore does not restore the encoded object type and instead shows whichever query is currently persisted on records.vi-table. Initialize the view query from the route parameter or avoid emitting a misleading type-specific URL.
Useful? React with 👍 / 👎.
| sourceRefs: stringList(sourceValue(source, 'sourceRefs', 'source_refs')), | ||
| routeDecision: sourceValue(source, 'routeDecision', 'route_decision'), | ||
| provenanceNodeId: text(sourceValue(source, 'provenanceNodeId', 'provenance_node_id')) || undefined, | ||
| provenanceNodeId: text(sourceValue(source, 'provenanceNodeId', 'provenance_node_id'), observedKey) || observedKey, |
There was a problem hiding this comment.
When an observed field omits provenanceNodeId but includes eventIds, this fallback assigns its semantic observedKey as though it were a graph node ID. ModelInspector then selects that value before eventIds[0] and sends it to WhyTrace, so declared fields backed by such observations request a nonexistent node instead of the available event provenance. Leave provenanceNodeId undefined when the server omits it so the existing event fallback remains reachable.
Useful? React with 👍 / 👎.
| // Canceling the form event keeps runnable commands local to the shared | ||
| // action sheet instead of leaking them into the chat transport. | ||
| event.preventDefault(); | ||
| openActionSheet({ instruction, chips: [] }); |
There was a problem hiding this comment.
When the composer already shows staged object references and the user submits /do ..., this opens the shared action sheet with an empty chip list. The visible staged references therefore remain behind the dialog but are omitted from the action pack and any delegated run, despite the action-sheet contract requiring visible context to match what travels. Convert the current staged refs into action-sheet chips when intercepting the command.
Useful? React with 👍 / 👎.
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/src/lib/thread-submit.ts`:
- Around line 6-10: Update actionInstructionFromThreadText to recognize /do only
when followed by whitespace or end-of-input, replacing the word-boundary
matching in both validation and removal logic. Add rejection cases for /do-not
and /do! in apps/console/src/lib/thread-submit.test.ts around lines 18-22, while
preserving valid action parsing.
In `@apps/console/src/views/survey/SurveyScene3D.tsx`:
- Around line 137-161: Update the edge button accessibility handling in the
SurveyScene3D edge render so inactive edges remain keyboard reachable and can be
pinned. Remove the inactive-state aria-hidden and tabIndex/pointer-event
restrictions, and ensure the DOM target remains visibly available or provide an
equivalent focusable keyboard control while preserving the existing click
pinning behavior.
🪄 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: 7b75f33e-d8d6-4bab-a4e2-197f9e03c817
⛔ Files ignored due to path filters (4)
apps/console/e2e/appearance.spec.ts-snapshots/workspace-1280-light-linux.pngis excluded by!**/*.pngapps/console/e2e/appearance.spec.ts-snapshots/workspace-1440-light-linux.pngis excluded by!**/*.pngapps/console/e2e/cards.spec.ts-snapshots/card-compact-inspector-linux.pngis excluded by!**/*.pngapps/console/e2e/cards.spec.ts-snapshots/cards-grid-linux.pngis excluded by!**/*.png
📒 Files selected for processing (11)
apps/console/e2e/hunk-review.spec.tsapps/console/e2e/survey.spec.tsapps/console/src/components/chat/RuntimeComposer.tsxapps/console/src/components/shell/IntuiShell.tsxapps/console/src/components/shell/Sidebar.tsxapps/console/src/components/ui/3d-image-gallery.tsxapps/console/src/lib/soft-navigate.test.tsapps/console/src/lib/soft-navigate.tsapps/console/src/lib/thread-submit.test.tsapps/console/src/lib/thread-submit.tsapps/console/src/views/survey/SurveyScene3D.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/console/src/components/shell/IntuiShell.tsx
- apps/console/src/components/shell/Sidebar.tsx
| export function actionInstructionFromThreadText(rawText: string): string | null { | ||
| const text = rawText.trim(); | ||
| if (!/^\/do\b/i.test(text)) return null; | ||
| return text.replace(/^\/do\b/i, '').trim(); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Parse /do only when followed by whitespace or end-of-input. The word-boundary check accepts punctuation, causing text such as /do-not to be routed as an action.
apps/console/src/lib/thread-submit.ts#L6-L10: replace\bwith a whitespace-or-end delimiter check.apps/console/src/lib/thread-submit.test.ts#L18-L22: add/do-notand/do!rejection cases.
📍 Affects 2 files
apps/console/src/lib/thread-submit.ts#L6-L10(this comment)apps/console/src/lib/thread-submit.test.ts#L18-L22
🤖 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/thread-submit.ts` around lines 6 - 10, Update
actionInstructionFromThreadText to recognize /do only when followed by
whitespace or end-of-input, replacing the word-boundary matching in both
validation and removal logic. Add rejection cases for /do-not and /do! in
apps/console/src/lib/thread-submit.test.ts around lines 18-22, while preserving
valid action parsing.
| pointerEvents={active ? 'auto' : 'none'} | ||
| style={{ pointerEvents: active ? 'auto' : 'none' }} | ||
| > | ||
| <div | ||
| className="survey-edge-reason flex h-6 items-center rounded-full bg-ij-editor px-2 text-xs text-ij-ink" | ||
| <button | ||
| type="button" | ||
| aria-label={`Pin connection: ${edge.reason}`} | ||
| aria-pressed={edgePinned} | ||
| aria-hidden={active ? undefined : true} | ||
| tabIndex={active ? 0 : -1} | ||
| className="survey-edge-reason flex h-6 appearance-none items-center rounded-full border-0 bg-ij-editor px-2 text-xs text-ij-ink" | ||
| data-survey-edge-id={edge.id} | ||
| data-edge-active={active ? 'true' : 'false'} | ||
| data-edge-emphasis={emphasis} | ||
| data-edge-pinned={edgePinned ? 'true' : 'false'} | ||
| data-edge-from={edge.from} | ||
| data-edge-to={edge.to} | ||
| data-edge-idle-opacity={idleOpacity} | ||
| aria-hidden="true" | ||
| style={{ opacity: active ? 1 : 0 }} | ||
| onClick={(event) => { | ||
| event.stopPropagation(); | ||
| setPinnedEdgeId((current) => current === edge.id ? null : edge.id); | ||
| }} | ||
| style={{ | ||
| opacity: active ? 1 : 0, | ||
| pointerEvents: active ? 'auto' : 'none', | ||
| }} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make inactive edges keyboard reachable.
When an edge is inactive, this button is aria-hidden, removed from tab order, and has pointer events disabled. The only remaining pin target is the Three.js mesh, which is not keyboard focusable. Keyboard users therefore cannot pin an edge initially. Provide a visible, focusable DOM target for inactive edges or an equivalent keyboard-accessible canvas control.
🤖 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/views/survey/SurveyScene3D.tsx` around lines 137 - 161,
Update the edge button accessibility handling in the SurveyScene3D edge render
so inactive edges remain keyboard reachable and can be pinned. Remove the
inactive-state aria-hidden and tabIndex/pointer-event restrictions, and ensure
the DOM target remains visibly available or provide an equivalent focusable
keyboard control while preserving the existing click pinning behavior.
💡 Codex Review Route viewer icons through the canonical icon ladder
The new JSON viewer imports Lucide glyphs directly, and the same bypass appears in the added log viewer and UI primitives. This skips the repository's normalized AGENTS.md reference: apps/console/AGENTS.md:L117-L117 CommonPlace/apps/console/src/components/log-viewer.tsx Lines 14 to 16 in 8013bb5 Replace prohibited dash characters in code comments
This added component introduces em dashes in several comments, including the export list and color-scale documentation, even though the scoped writing rule bans em and en dashes everywhere, explicitly including code comments. Replace them with permitted punctuation. AGENTS.md reference: apps/console/AGENTS.md:L161-L164 CommonPlace/apps/console/src/lib/console-host.ts Lines 478 to 481 in 8013bb5 Abort remote migration when an action receipt is refused
Fresh evidence after the earlier layout-migration comments is that this replacement now emits creates and moves but ignores every returned AGENTS.md reference: apps/console/AGENTS.md:L145-L150 Match pathname before hard-falling back for query URLs
When an object-type row supplies this CommonPlace/apps/console/src/views/survey/SurveyScene3D.tsx Lines 140 to 145 in 8013bb5 Keep edge labels active during their own interaction
When an unpinned connection becomes active only because its Three.js mesh or related card is hovered, moving the pointer onto this new HTML button fires the source mesh's pointer-out and clears the state that makes the button interactive. The button then switches to ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d6d4fa265
ℹ️ 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".
| if (options.hardFallback) { | ||
| window.location.assign(target); | ||
| return; |
There was a problem hiding this comment.
When an Objects row calls this helper with /records?type=..., target retains the query string while every completion check reads only window.location.pathname, which can equal only /records. The check therefore never succeeds, and the newly enabled hardFallback forces a full-page reload four seconds after every otherwise-successful object-type navigation. Compare the pathname portion of the target URL while preserving the full URL for router.push and the fallback.
Useful? React with 👍 / 👎.
| export const threadStagedAtom = atomWithStorage<StagedThreadRef[]>( | ||
| 'commonplace.console.thread-staged.v1', | ||
| [], | ||
| threadSessionStorage, | ||
| { getOnInit: true }, |
There was a problem hiding this comment.
If one person stages object references, signs out, and another account signs in within the same browser tab, this origin-wide sessionStorage key restores the first account's labels and theorem addresses for the second account. threadActions.send then appends those restored references to the new account's prompt, and the sign-out path never clears them. Namespace this state by the verified tenant/account or clear it whenever the authenticated identity changes instead of persisting user context in an unscoped browser key.
AGENTS.md reference: apps/console/AGENTS.md:L173-L173
Useful? React with 👍 / 👎.
Resolve post-#110 conflicts favoring the fork vector/layout architecture while retaining GraphSnapshotSource find bounds and longer survey poll timeout. Co-authored-by: Travis Gilbert <Travis-Gilbert@users.noreply.github.com>
Keep URL-first Place navigation from #135 inside the post-fork split shortcut listeners, and preserve the hardened browser readiness waits. Co-authored-by: Travis Gilbert <Travis-Gilbert@users.noreply.github.com>
Summary by CodeRabbit