feat(console): ARD Parts 1/4/6 UI remaining (excl WorkOS) - #201
Conversation
Wire the data-model settings register to /rest/metadata, gate program Run on validate, bind commands gallery to substrate gallery/gallery_fork, and close D20–D23 / D33–D36 with named blockers. WorkOS stays deferred.
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
📝 WalkthroughWalkthroughThis PR adds the ARD theorem UI implementation records, Twenty metadata contracts, metadata REST handling, model settings screens, program validation, command gallery integration, and supporting console routes and surface wiring. ChangesARD theorem UI
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ModelSettingsView
participant MetadataClient
participant MetadataRoute
participant MetadataREST
participant MetadataStore
User->>ModelSettingsView: Edit object, field, index, or facet
ModelSettingsView->>MetadataClient: Submit metadata mutation
MetadataClient->>MetadataRoute: Send same-origin REST request
MetadataRoute->>MetadataREST: Forward metadata request
MetadataREST->>MetadataStore: Use local fallback when needed
MetadataStore-->>MetadataClient: Return updated metadata
MetadataClient-->>ModelSettingsView: Refresh settings state
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds the remaining ARD Parts 1/4/6 console UI slices (excluding WorkOS) by introducing a Twenty-shaped metadata wire + settings register UI, wiring command gallery + gallery fork through programmable_graph, and gating Program execution behind pre-run validation while improving records refusal UX and adding local-dev metadata REST fallbacks + smoke tooling.
Changes:
- Introduces Twenty metadata wire contracts (
twenty-metadata) and exposes them via@commonplace/data-model-contracts. - Adds
/Data-model/settingssettings register UI backed by same-origin/api/rest/metadata/*proxy +LocalDevMetadataStorefallback. - Extends Program + Commands flows with
validate+gallery/gallery_forkclient support and updates routing/seeding for additional surfaces.
Reviewed changes
Copilot reviewed 41 out of 41 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/data-model-contracts/src/twenty-metadata.ts | Adds Twenty-shaped metadata wire types and adapters between Twenty tokens and canonical FieldType. |
| packages/data-model-contracts/src/twenty-metadata.test.ts | Unit tests for Twenty token parsing and FieldType adapter behavior. |
| packages/data-model-contracts/src/index.ts | Re-exports Twenty metadata wire types + helpers from the contracts package. |
| apps/console/src/views/registry.tsx | Registers the new Model Settings view and updates Goal Stack labeling/usage text. |
| apps/console/src/views/RecordTableView.tsx | Updates edit refusal detection to surface validation refusal codes/notes. |
| apps/console/src/views/program/RunRail.tsx | Adds Validate UI and displays validation summaries + node IDs. |
| apps/console/src/views/program/ProgramView.tsx | Gates Run behind validate and wires validation state into the RunRail. |
| apps/console/src/views/program/programClient.ts | Adds validateProgramDefinition, command gallery fetch + gallery_fork client, and helpers. |
| apps/console/src/views/program/programClient.gallery.test.ts | Tests for gallery mapping/fallback and validate refusal parsing/blocking logic. |
| apps/console/src/views/model/settings/ModelSettingsView.tsx | New four-screen data model settings register UI (objects/fields/indexes/facets). |
| apps/console/src/views/model/settings/metadataClient.ts | Client for same-origin /api/rest/metadata/* endpoints. |
| apps/console/src/views/model/settings/FieldSettingsPanels.tsx | Field/index/facet panel components used by the settings register. |
| apps/console/src/views/model/ModelView.tsx | Updates Models surface shell/layout behavior and adds link to settings register. |
| apps/console/src/views/CommandsGalleryView.tsx | Replaces fixture-only gallery with substrate gallery + gallery_fork wiring and LocalDev fallback labeling. |
| apps/console/src/lib/workspace-seed.ts | Seeds new surfaces/view-instances (model-settings/search/kanban/commands) and adjusts goal surface metadata. |
| apps/console/src/lib/view-routing-retirement.test.ts | Expands routing tests for additional surface routes. |
| apps/console/src/lib/surface-routes.ts | Adds auxiliary routes for goals/search/kanban/commands/model-settings. |
| apps/console/src/lib/server/metadata-rest.ts | Server-side proxy/forwarder for harness /rest/metadata/* with LocalDev fallback. |
| apps/console/src/lib/server/local-dev-metadata-store.ts | Implements in-process LocalDev stand-in store for metadata REST operations. |
| apps/console/src/lib/server/local-dev-metadata-store.test.ts | Node-environment tests covering LocalDev metadata store behavior. |
| apps/console/src/lib/rail/block-surface-targets.ts | Adds palette-item → surface-route resolution helper. |
| apps/console/src/lib/rail/block-surface-targets.test.ts | Tests for palette-item → surface-route resolution helper. |
| apps/console/src/app/goals/page.tsx | Adds App Router page for the goals surface. |
| apps/console/src/app/Data-model/settings/page.tsx | Adds App Router page for /Data-model/settings. |
| apps/console/src/app/commands/page.tsx | Adds App Router page for the commands surface. |
| apps/console/src/app/api/rest/metadata/[[...path]]/route.ts | Adds same-origin API route to proxy metadata REST requests. |
| apps/console/scripts/smoke-metadata-rest.mjs | Adds env-gated live smoke script for harness metadata REST. |
| apps/console/CLAUDE.md | Updates console component ledger documentation to include the settings register. |
| apps/console/AGENTS.md | Updates console component ledger documentation (model canvas + settings register entries). |
| .full-stack-feature/state.json | Adds feature state tracking for this multi-part ARD UI effort. |
| .full-stack-feature/SPEC-REVIEW.md | Adds spec review report mapping ARD requirements to implementation status. |
| .full-stack-feature/research-d32-d36-seams.md | Adds seam inventory notes for D32–D36 (WorkOS excluded). |
| .full-stack-feature/research-d20-d23-seams.md | Adds seam inventory notes for D20–D23. |
| .full-stack-feature/EXECUTE-REPORT-WAVE-B.md | Adds execute report for Wave B (gallery + validate). |
| .full-stack-feature/EXECUTE-REPORT-WAVE-A.md | Adds execute report for Wave A (settings register). |
| .full-stack-feature/EXECUTE-REPORT-REMAINING.md | Adds consolidated execute report for remaining ARD UI work (excluding WorkOS). |
| .full-stack-feature/EXECUTE-CHECKLIST-WAVE-B.md | Adds Wave B checklist artifact. |
| .full-stack-feature/EXECUTE-CHECKLIST-WAVE-A.md | Adds Wave A checklist artifact. |
| .full-stack-feature/ard-ui-remaining-checklist.json | Adds portable fallback checklist JSON for ARD UI remaining work. |
| .full-stack-feature/ard-ui-remaining-1.0--fallback-20260808a.md | Adds fallback plan markdown for ARD UI remaining work. |
| .full-stack-feature/00-research-map.md | Adds/updates research map and execution wave planning for ARD UI. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| { kind: 'goals', path: '/goals', surfaceId: 'console-goals', tier: 'place' as const }, | ||
| { kind: 'search', path: '/search', surfaceId: 'console-search', tier: 'place' as const }, | ||
| { kind: 'kanban', path: '/kanban', surfaceId: 'console-kanban', tier: 'place' as const }, | ||
| { kind: 'commands', path: '/commands', surfaceId: 'console-commands', tier: 'place' as const }, | ||
| { kind: 'model-settings', path: '/Data-model/settings', surfaceId: 'console-model-settings', tier: 'place' as const }, |
| <div className="flex h-ij-toolbar shrink-0 items-center gap-3 border-b border-ij-seam px-3"> | ||
| <Link | ||
| href="/Data-model/settings" | ||
| className="text-sm text-ij-ink-info underline-offset-2 hover:text-ij-ink hover:underline" | ||
| > | ||
| Settings register | ||
| </Link> | ||
| <span className="text-xs text-ij-ink-info"> | ||
| Object types · fields · indexes · facet conformance | ||
| </span> | ||
| </div> |
| const checks = Array.isArray(validation?.checks) | ||
| ? validation!.checks!.flatMap((item) => { | ||
| if (!item || typeof item !== 'object' || Array.isArray(item)) return []; | ||
| const row = item as Record<string, unknown>; | ||
| return typeof row.requirement === 'string' ? [row.requirement] : []; | ||
| }) | ||
| : []; | ||
| const validationPassed = checks.length === 0 | ||
| ? true | ||
| : checks.every((_, index) => { | ||
| const item = (validation!.checks as unknown[])[index]; | ||
| if (!item || typeof item !== 'object' || Array.isArray(item)) return true; | ||
| return (item as { passed?: boolean }).passed !== false; | ||
| }); |
| const showEnum = type === 'SELECT' || type === 'MULTI_SELECT'; | ||
| const showVector = type === 'RAW_JSON'; | ||
| const showRelation = type === 'RELATION' || type === 'MORPH_RELATION'; |
| // SOURCING: none. Maps Blocks palette rows to page-owned surfaces so sidebar | ||
| // clicks navigate instead of dumping view-instances into the center island. | ||
|
|
||
| import { SURFACE_ROUTES } from '@/lib/surface-routes'; | ||
| import type { BlockPaletteItem } from '@/lib/rail/rail-model'; |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/console/src/views/program/ProgramView.tsx (1)
1162-1203: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDiscard validation results for a changed draft.
A user can switch programs or edit the definition while
validateProgramDefinitionis pending. The completion then updates validation state for the new draft. InrunProgram, it can also execute the old captureddefinitionafter the user has moved to another program.Capture
draftGeneration.currentand the definition before validation. After the await, stop when the generation ordefinitionRef.currenthas changed. Run only the validated snapshot.🤖 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/program/ProgramView.tsx` around lines 1162 - 1203, Update runProgram and validateProgram to capture the current draft generation and definition snapshot before awaiting validateProgramDefinition, then discard the result if draftGeneration.current or definitionRef.current changed. In runProgram, execute runProgramDefinition only with the validated snapshot, and stop without updating validation or execution state when the draft is stale.
🧹 Nitpick comments (7)
apps/console/src/lib/view-routing-retirement.test.ts (1)
27-36: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression coverage for all new route and resolver branches.
The new surface contract is only partially covered by tests.
- apps/console/src/lib/view-routing-retirement.test.ts#L27-L36: Add forward assertions for
/kanban,/commands, and/Data-model/settings, plus the reverse assertion forconsole-model-settings.- apps/console/src/lib/rail/block-surface-targets.test.ts#L6-L35: Exercise palette-kind lookup, item-ID lookup, and nested basename fallback for
/Data-model/settings.🤖 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/view-routing-retirement.test.ts` around lines 27 - 36, Add regression assertions in apps/console/src/lib/view-routing-retirement.test.ts:27-36 for forward resolution of /kanban, /commands, and /Data-model/settings, plus reverse resolution of console-model-settings. In apps/console/src/lib/rail/block-surface-targets.test.ts:6-35, cover palette-kind lookup, item-ID lookup, and nested basename fallback for /Data-model/settings.apps/console/src/views/model/settings/FieldSettingsPanels.tsx (1)
429-432: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winType-based auto-mapping can bind the wrong field.
The fallback at line 431 matches the first field with the same
type. Two unrelatedTEXTfields make this selection arbitrary. The draft map is then submitted directly by "Apply conformance". Consider leaving type-only matches unmapped so the user must choose, which matches the guidance text above the list.🤖 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/model/settings/FieldSettingsPanels.tsx` around lines 429 - 432, Update the auto-mapping logic in the property-to-field mapping block to remove the type-only fallback from object.fields.find. Only map properties when their names match exactly; leave unmatched or type-only matches unmapped so users can select the correct field before applying conformance.packages/data-model-contracts/src/twenty-metadata.test.ts (1)
33-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for
editorStateFromFieldandindexPolicyFromSettings.Both functions are exported and consumed by the settings panels, but no test covers them. A test that loads a
MULTI_SELECTfield carryingsettings.raw.fieldTypewould catch the token downgrade reported ontwenty-metadata.tslines 140-159.🤖 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/data-model-contracts/src/twenty-metadata.test.ts` around lines 33 - 57, Extend the tests near fieldTypeFromEditor to cover the exported editorStateFromField and indexPolicyFromSettings functions. Include a MULTI_SELECT field whose settings.raw.fieldType preserves its original token, and assert the resulting editor state and index policy so token downgrades are detected.apps/console/src/views/model/ModelView.tsx (1)
907-932: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused restore helper.
restoreRightVersionhas no callers, soonRestoreVersionshould drive the restore flow instead or both handlers should use a sharedrestoreVersion(id: string)helper.🤖 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/model/ModelView.tsx` around lines 907 - 932, The restore flow should not retain an unused restoreRightVersion helper. Consolidate its behavior into a shared restoreVersion(id: string) helper used by onRestoreVersion, or remove the helper and keep the complete restore logic directly in onRestoreVersion, ensuring no dead handler remains.apps/console/src/lib/server/local-dev-metadata-store.test.ts (1)
18-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider covering demotion and refusal paths.
The suite covers promotion but not
DELETE /indexes/:object/:field, the system-fieldPATCHrefusal (403), and the unknown-route 404. Those are the branches most likely to regress in the stand-in.🤖 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/server/local-dev-metadata-store.test.ts` around lines 18 - 46, Extend the local metadata store tests around handleLocalDevMetadata to cover DELETE /indexes/:object/:field demotion, PATCH attempts on system fields returning 403, and unknown routes returning 404. Assert each response status and verify demotion restores the promotion candidate where applicable.apps/console/src/lib/server/local-dev-metadata-store.ts (1)
259-272: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
PATCHcannot cleardescription.
patch.description ?? existing.descriptionkeeps the old value when the client sendsnull. Use a key presence check if clearing must be supported. The same pattern appears at line 400 for fields.🤖 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/server/local-dev-metadata-store.ts` around lines 259 - 272, Update the PATCH handling in the object metadata update flow to distinguish an omitted description from an explicitly provided null, allowing clients to clear description instead of retaining existing.description. Apply the same key-presence-based merge behavior to the fields update logic around the corresponding PATCH handling, while preserving existing values when those keys are omitted.apps/console/src/lib/server/metadata-rest.ts (1)
42-59: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a timeout to the upstream fetch.
The
fetchcall has no timeout. If the harness node accepts the connection and never responds, the route handler blocks until the platform kills the request. Add anAbortSignal.timeout(...)so the local fallback engages.♻️ Proposed change
body: bodyText, cache: 'no-store', + signal: AbortSignal.timeout(10_000), });🤖 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/server/metadata-rest.ts` around lines 42 - 59, Update the upstream fetch in the metadata request handler to include an AbortSignal.timeout(...) option, using the existing timeout configuration or an appropriate bounded duration. Ensure timeout failures are caught by the current catch block so handleLocalDevMetadata(method, segments, bodyText) runs.
🤖 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 @.full-stack-feature/ard-ui-remaining-1.0--fallback-20260808a.md:
- Around line 22-35: Synchronize the task markers in the checklist with the
corresponding entries in ard-ui-remaining-checklist.json: update each task
marked done there from [ ] to [x], or use the project’s explicit
closed-with-blocker marker where applicable. Preserve task names and ordering so
follow-up executors do not repeat completed work.
In @.full-stack-feature/ard-ui-remaining-checklist.json:
- Around line 155-180: Align the metadata-live-smoke state by either changing
status from done to pending until live proof runs, or changing
live_oracle_required to false if the env-gated script is the only required
deliverable. Update the related evidence or deferral note so it accurately
reflects the selected requirement and the skipped-live-proof state.
In @.full-stack-feature/SPEC-REVIEW.md:
- Around line 1-6: Update the review document’s status metadata to identify it
as a pre-remediation baseline, including an as-of commit or timestamp. Link to
the final execution report, or regenerate the requirement mapping so findings
such as D24 gating, tagged FieldType forms, facet mapping, and P3 demotion
reflect the final implementation state.
In @.full-stack-feature/state.json:
- Around line 9-21: Remove the final execution entry from completed_steps in
.full-stack-feature/state.json until all merge-blocking gates have evidence. In
.full-stack-feature/00-research-map.md lines 70-79, replace “Gates DONE” with
the exact completed gate set; in .full-stack-feature/EXECUTE-REPORT-REMAINING.md
lines 43-50, record results for import fence, register lint, contrast, motion
inventory, icon paint, canonical checkout, and the Playwright visual baseline,
leaving execution blocked or incomplete while any result is missing.
In `@apps/console/src/lib/rail/block-surface-targets.ts`:
- Around line 63-70: The fallback lookup in the SURFACE_ROUTES find predicate
must compare the basename of each route path with item.id or item.kind, not only
the complete path. Derive the final path segment from entry.path and use it
alongside the existing surfaceId checks, preserving the current matching
behavior.
In `@apps/console/src/lib/server/local-dev-metadata-store.ts`:
- Around line 456-466: Update the DELETE branch handling index removal to also
reset the target field’s isFilterable and isSortable flags to false, while
preserving unrelated field metadata and the existing index removal response.
Locate the field metadata within object and persist the updated object together
with the filtered indexMetadataList.
- Around line 1-3: Update the comment for LocalDevMetadataStore to replace the
em dash with a colon or period, preserving the existing meaning and wording
otherwise.
- Around line 44-65: Update the field metadata construction around the opts
spread so partial opts.settings cannot overwrite normalized settings flags.
Preserve caller options while merging opts.settings with the defaults for
isFilterable and isSortable, ensuring createLocalFieldMetadata always returns
complete FieldSettingsWire data for incoming request bodies.
In `@apps/console/src/lib/server/metadata-rest.ts`:
- Around line 30-32: Update the fallback branch in the metadata request handler
to prevent unauthenticated production access: ensure resolveHarnessPrincipal()
and its session/tenant authorization checks run before handleLocalDevMetadata,
or only allow the local fallback when process.env.NODE_ENV is not production.
Preserve the existing local development behavior while blocking reads and
mutations in deployed environments without configured metadata URLs.
- Around line 62-76: The stand-in metadata banner is only initialized during the
initial reload, so ModelSettingsView sub-screens can display fallback data
without the banner. Update ModelSettingsView to initialize the
LocalDevMetadataStore source banner for every screen load, or include the active
screen source in the banner text while preserving the existing stand-in metadata
behavior.
In `@apps/console/src/lib/workspace-seed.ts`:
- Around line 318-329: Update the seed definitions for the primary place
surfaces `Model settings`, `Search`, `Kanban`, and `Commands` to include
explicit `stripe_order` values. Preserve the existing ordering scheme and assign
deterministic values consistent with the intended stripe placement, rather than
relying on the `?? 99` fallback.
In `@apps/console/src/views/model/ModelView.tsx`:
- Around line 692-695: Adjust the declaration success flow in ModelView,
including spawnObject and the reload effect, so advancing reloadToken does not
immediately clear the newly set notice. Ensure the success message is applied
after the reload completes, or preserve it for reloads triggered by the local
declaration write, while retaining notice clearing for other reloads.
In `@apps/console/src/views/model/settings/FieldSettingsPanels.tsx`:
- Around line 102-104: Update the field settings logic around showEnum and
fieldTypeFromEditor so MULTI_SELECT no longer exposes an enum variant input that
cannot be persisted. Restrict showEnum to SELECT only, leaving MULTI_SELECT
handled by its existing JSON type path and preserving the SELECT enum behavior.
In `@apps/console/src/views/model/settings/ModelSettingsView.tsx`:
- Line 432: Replace all four em dashes with the specified punctuation: update
ModelSettingsView.tsx lines 432 and 3-6, FieldSettingsPanels.tsx lines 441-444,
and registry.tsx lines 808-809. Preserve the surrounding text while using
periods, a colon, and a comma as specified.
In `@apps/console/src/views/program/programClient.gallery.test.ts`:
- Line 5: Update the comment above the client helper tests to replace the em
dash with a period or colon, preserving the comment’s meaning and avoiding em or
en dashes.
In `@apps/console/src/views/program/programClient.ts`:
- Around line 421-475: Update validateProgramDefinition so a successful
validation response returns ok: false when nested.checks is not an array, with
an appropriate validation-failure code/message and no receipt. Preserve the
existing successful path for an explicit empty checks array and for valid check
entries.
In `@apps/console/src/views/program/ProgramView.tsx`:
- Around line 291-292: Update resetRunState to clear both validationSummary and
validationNodeIds whenever the active program changes, alongside its existing
run-state resets. Preserve the existing reset behavior for openProgram,
openStarter, forkProgram, and proposal flows.
In `@packages/data-model-contracts/src/twenty-metadata.ts`:
- Around line 140-159: In the field-type normalization block, preserve the
original wire token in `token` when it agrees with the canonical `FieldType`
kind, and only use `fieldTypeToTwentyToken(typed)` when `field.type` disagrees
with that kind. Update the logic around `canonical`, `typed`, and `token` so
compound wire tokens such as MULTI_SELECT, ARRAY, EMAILS, PHONES, LINKS,
CURRENCY, RATING, NUMERIC, and RICH_TEXT are not rewritten after saving, while
existing enum, vector, and relation metadata handling remains intact.
---
Outside diff comments:
In `@apps/console/src/views/program/ProgramView.tsx`:
- Around line 1162-1203: Update runProgram and validateProgram to capture the
current draft generation and definition snapshot before awaiting
validateProgramDefinition, then discard the result if draftGeneration.current or
definitionRef.current changed. In runProgram, execute runProgramDefinition only
with the validated snapshot, and stop without updating validation or execution
state when the draft is stale.
---
Nitpick comments:
In `@apps/console/src/lib/server/local-dev-metadata-store.test.ts`:
- Around line 18-46: Extend the local metadata store tests around
handleLocalDevMetadata to cover DELETE /indexes/:object/:field demotion, PATCH
attempts on system fields returning 403, and unknown routes returning 404.
Assert each response status and verify demotion restores the promotion candidate
where applicable.
In `@apps/console/src/lib/server/local-dev-metadata-store.ts`:
- Around line 259-272: Update the PATCH handling in the object metadata update
flow to distinguish an omitted description from an explicitly provided null,
allowing clients to clear description instead of retaining existing.description.
Apply the same key-presence-based merge behavior to the fields update logic
around the corresponding PATCH handling, while preserving existing values when
those keys are omitted.
In `@apps/console/src/lib/server/metadata-rest.ts`:
- Around line 42-59: Update the upstream fetch in the metadata request handler
to include an AbortSignal.timeout(...) option, using the existing timeout
configuration or an appropriate bounded duration. Ensure timeout failures are
caught by the current catch block so handleLocalDevMetadata(method, segments,
bodyText) runs.
In `@apps/console/src/lib/view-routing-retirement.test.ts`:
- Around line 27-36: Add regression assertions in
apps/console/src/lib/view-routing-retirement.test.ts:27-36 for forward
resolution of /kanban, /commands, and /Data-model/settings, plus reverse
resolution of console-model-settings. In
apps/console/src/lib/rail/block-surface-targets.test.ts:6-35, cover palette-kind
lookup, item-ID lookup, and nested basename fallback for /Data-model/settings.
In `@apps/console/src/views/model/ModelView.tsx`:
- Around line 907-932: The restore flow should not retain an unused
restoreRightVersion helper. Consolidate its behavior into a shared
restoreVersion(id: string) helper used by onRestoreVersion, or remove the helper
and keep the complete restore logic directly in onRestoreVersion, ensuring no
dead handler remains.
In `@apps/console/src/views/model/settings/FieldSettingsPanels.tsx`:
- Around line 429-432: Update the auto-mapping logic in the property-to-field
mapping block to remove the type-only fallback from object.fields.find. Only map
properties when their names match exactly; leave unmatched or type-only matches
unmapped so users can select the correct field before applying conformance.
In `@packages/data-model-contracts/src/twenty-metadata.test.ts`:
- Around line 33-57: Extend the tests near fieldTypeFromEditor to cover the
exported editorStateFromField and indexPolicyFromSettings functions. Include a
MULTI_SELECT field whose settings.raw.fieldType preserves its original token,
and assert the resulting editor state and index policy so token downgrades are
detected.
🪄 Autofix
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: d3f42b85-add2-48ab-90b8-d4f904f15737
📒 Files selected for processing (41)
.full-stack-feature/00-research-map.md.full-stack-feature/EXECUTE-CHECKLIST-WAVE-A.md.full-stack-feature/EXECUTE-CHECKLIST-WAVE-B.md.full-stack-feature/EXECUTE-REPORT-REMAINING.md.full-stack-feature/EXECUTE-REPORT-WAVE-A.md.full-stack-feature/EXECUTE-REPORT-WAVE-B.md.full-stack-feature/SPEC-REVIEW.md.full-stack-feature/ard-ui-remaining-1.0--fallback-20260808a.md.full-stack-feature/ard-ui-remaining-checklist.json.full-stack-feature/research-d20-d23-seams.md.full-stack-feature/research-d32-d36-seams.md.full-stack-feature/state.jsonapps/console/AGENTS.mdapps/console/CLAUDE.mdapps/console/scripts/smoke-metadata-rest.mjsapps/console/src/app/Data-model/settings/page.tsxapps/console/src/app/api/rest/metadata/[[...path]]/route.tsapps/console/src/app/commands/page.tsxapps/console/src/app/goals/page.tsxapps/console/src/lib/rail/block-surface-targets.test.tsapps/console/src/lib/rail/block-surface-targets.tsapps/console/src/lib/server/local-dev-metadata-store.test.tsapps/console/src/lib/server/local-dev-metadata-store.tsapps/console/src/lib/server/metadata-rest.tsapps/console/src/lib/surface-routes.tsapps/console/src/lib/view-routing-retirement.test.tsapps/console/src/lib/workspace-seed.tsapps/console/src/views/CommandsGalleryView.tsxapps/console/src/views/RecordTableView.tsxapps/console/src/views/model/ModelView.tsxapps/console/src/views/model/settings/FieldSettingsPanels.tsxapps/console/src/views/model/settings/ModelSettingsView.tsxapps/console/src/views/model/settings/metadataClient.tsapps/console/src/views/program/ProgramView.tsxapps/console/src/views/program/RunRail.tsxapps/console/src/views/program/programClient.gallery.test.tsapps/console/src/views/program/programClient.tsapps/console/src/views/registry.tsxpackages/data-model-contracts/src/index.tspackages/data-model-contracts/src/twenty-metadata.test.tspackages/data-model-contracts/src/twenty-metadata.ts
| - [ ] **d24-gate-run** (build.afk) — D24 gate Run on validate | ||
| - [ ] **fieldtype-forms** (build.afk) — Tagged FieldType field editor forms | ||
| - [ ] **facet-manual-map** (build.afk) — Manual facet property mapping | ||
| - [ ] **index-policy-editor** (build.afk) — IndexPolicy four flags + provenance | ||
| - [ ] **metadata-live-smoke** (build.afk) — Env-gated live metadata REST smoke | ||
| - [ ] **records-validated-ux** (build.afk) — Records put_item_validated refusal UX | ||
| - [ ] **p3-goal-stack-demote** (build.afk) — P3 demote Goal Stack | ||
| - [ ] **gallery-substrate-fill** (build.afk) — Commands gallery substrate fill | ||
| - [ ] **layer-chrome** (build.afk) — D1 Layer 0/1/2 chrome on models settings | ||
| - [ ] **fabric-seam-inventory** (decision.afk) — Inventory D20-D23 substrate seams | ||
| - [ ] **fabric-ui-or-close** (build.afk) — Ship or close D20-D23 UI | ||
| - [ ] **wave-c-seams** (decision.afk) — Inventory D32-D36 seams (excl WorkOS) | ||
| - [ ] **wave-c-ui-or-close** (build.afk) — Ship or close D32-D36 UI (excl WorkOS) | ||
| - [ ] **settings-twenty-inputs** (build.afk) — Replace native settings inputs with twenty-ui |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Synchronize the fallback task markers.
Every task remains unchecked here. .full-stack-feature/ard-ui-remaining-checklist.json marks the corresponding tasks as done.
Mark completed tasks as [x], or add an explicit closed-with-blocker state. The current checklist can cause a follow-up executor to repeat completed work.
🤖 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 @.full-stack-feature/ard-ui-remaining-1.0--fallback-20260808a.md around lines
22 - 35, Synchronize the task markers in the checklist with the corresponding
entries in ard-ui-remaining-checklist.json: update each task marked done there
from [ ] to [x], or use the project’s explicit closed-with-blocker marker where
applicable. Preserve task names and ordering so follow-up executors do not
repeat completed work.
| "id": "metadata-live-smoke", | ||
| "title": "Env-gated live metadata REST smoke", | ||
| "status": "done", | ||
| "kind": "build.afk", | ||
| "node_type": "build", | ||
| "controller": "agent", | ||
| "gist": "Named live/env-gated smoke; stand-in never equals done.", | ||
| "oracle_class": "unit", | ||
| "implementation_mode": "local_process", | ||
| "evidence_class": "unit", | ||
| "substitution_allowed": false, | ||
| "live_oracle_required": true, | ||
| "dependencies": [], | ||
| "acceptance": [ | ||
| "An env-gated test or script exists and is documented.", | ||
| "Without the env URL, the test skips/ignores rather than claiming pass on LocalDev.", | ||
| "LocalDevMetadataStore remains explicitly named as stand-in." | ||
| ], | ||
| "proof_command": "test -f apps/console/scripts/smoke-metadata-rest.mjs || test -f apps/console/src/lib/server/metadata-rest.live.test.ts", | ||
| "routes": [ | ||
| "apps/console/src/lib/server/metadata-rest.ts", | ||
| "apps/console/scripts/smoke-metadata-rest.mjs" | ||
| ], | ||
| "evidence": "scripts/smoke-metadata-rest.mjs env-gated (skipped without URL)", | ||
| "deferral_reason": null, | ||
| "provenance": "SPEC-REVIEW SR-002 / ARD D2" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Make the live-oracle state consistent.
live_oracle_required is true, but this item is done with unit evidence while the final report says the live smoke did not run.
If live proof is required, set the item to pending until it runs. If the required deliverable is only the env-gated script, set live_oracle_required to false and retain the skipped-live-proof note.
🤖 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 @.full-stack-feature/ard-ui-remaining-checklist.json around lines 155 - 180,
Align the metadata-live-smoke state by either changing status from done to
pending until live proof runs, or changing live_oracle_required to false if the
env-gated script is the only required deliverable. Update the related evidence
or deferral note so it accurately reflects the selected requirement and the
skipped-live-proof state.
| # Spec Review: ARD-THEOREM-2026-08-04 Parts 1 / 4 / 6 (UI) | ||
|
|
||
| **Date:** 2026-08-08 | ||
| **Spec:** `Theorem/docs/ARD-THEOREM-2026-08-04 (1).md` | ||
| **Code scope:** CommonPlace console + Theorem MCP `programmable_graph` (Wave A + B1/B2) | ||
| **Runtime scope:** focused vitest only (no browser / deployed smoke) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Mark this review as a pre-remediation baseline.
This document presents superseded findings as current state. For example, Lines 62-98 state that D24 gating, tagged FieldType forms, facet mapping, and P3 demotion remain incomplete, while .full-stack-feature/ard-ui-remaining-checklist.json records those tasks as done.
Add an as-of commit or timestamp. Link to the final execution report. Alternatively, regenerate the requirement mapping from the final implementation state.
🤖 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 @.full-stack-feature/SPEC-REVIEW.md around lines 1 - 6, Update the review
document’s status metadata to identify it as a pre-remediation baseline,
including an as-of commit or timestamp. Link to the final execution report, or
regenerate the requirement mapping so findings such as D24 gating, tagged
FieldType forms, facet mapping, and P3 demotion reflect the final implementation
state.
| "completed_steps": [ | ||
| "00-research-map", | ||
| "wave-a-settings-register", | ||
| "wave-a-gate-twenty", | ||
| "wave-b1-commands-gallery", | ||
| "wave-b2-program-validate", | ||
| "spec-review", | ||
| "plan-ard-ui-remaining", | ||
| "execute-ard-ui-remaining" | ||
| ], | ||
| "out_of_scope": ["WorkOS AuthKit / next-auth provider swap"], | ||
| "notes": "Plan+execute remaining ARD UI except WorkOS. Substrate plan mint blocked by store size; fallback checklist used. D20-D23 and D33-D36 closed with named blockers.", | ||
| "last_updated": "2026-08-08T19:52:00.000Z" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Do not record final execution as complete before all merge gates have evidence.
The final report records gate:twenty only and says browser validation was not run. Update the execution state to blocked or incomplete until the required results exist.
.full-stack-feature/state.json#L9-L21: remove completed execution status until all required gate results are recorded..full-stack-feature/00-research-map.md#L70-L79: replaceGates DONEwith the exact completed gate set..full-stack-feature/EXECUTE-REPORT-REMAINING.md#L43-L50: add results for every required gate and the Playwright visual baseline.
Based on learnings: Run all merge-blocking gates: import fence, register lint, contrast, motion inventory, icon paint, canonical checkout, and Playwright visual baseline.
📍 Affects 3 files
.full-stack-feature/state.json#L9-L21(this comment).full-stack-feature/00-research-map.md#L70-L79.full-stack-feature/EXECUTE-REPORT-REMAINING.md#L43-L50
🤖 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 @.full-stack-feature/state.json around lines 9 - 21, Remove the final
execution entry from completed_steps in .full-stack-feature/state.json until all
merge-blocking gates have evidence. In .full-stack-feature/00-research-map.md
lines 70-79, replace “Gates DONE” with the exact completed gate set; in
.full-stack-feature/EXECUTE-REPORT-REMAINING.md lines 43-50, record results for
import fence, register lint, contrast, motion inventory, icon paint, canonical
checkout, and the Playwright visual baseline, leaving execution blocked or
incomplete while any result is missing.
Source: Learnings
| // Last resort: any SURFACE_ROUTES entry whose path basename matches the id. | ||
| const route = SURFACE_ROUTES.find( | ||
| (entry) => | ||
| entry.path === `/${item.id}` || | ||
| entry.path === `/${item.kind}` || | ||
| entry.surfaceId === `console-${item.id}` || | ||
| entry.surfaceId === `console-${item.kind}`, | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the fallback match nested route basenames.
The fallback description promises a basename match, but the predicate compares only complete paths. An item with an unknown descriptor and id or kind equal to settings therefore does not resolve /Data-model/settings. Compute the final path segment before comparing.
Proposed fix
- const route = SURFACE_ROUTES.find(
- (entry) =>
- entry.path === `/${item.id}` ||
- entry.path === `/${item.kind}` ||
- entry.surfaceId === `console-${item.id}` ||
- entry.surfaceId === `console-${item.kind}`,
- );
+ const route = SURFACE_ROUTES.find((entry) => {
+ const basename = entry.path.slice(entry.path.lastIndexOf('/') + 1);
+ return (
+ basename === item.id ||
+ basename === item.kind ||
+ entry.surfaceId === `console-${item.id}` ||
+ entry.surfaceId === `console-${item.kind}`
+ );
+ });📝 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.
| // Last resort: any SURFACE_ROUTES entry whose path basename matches the id. | |
| const route = SURFACE_ROUTES.find( | |
| (entry) => | |
| entry.path === `/${item.id}` || | |
| entry.path === `/${item.kind}` || | |
| entry.surfaceId === `console-${item.id}` || | |
| entry.surfaceId === `console-${item.kind}`, | |
| ); | |
| // Last resort: any SURFACE_ROUTES entry whose path basename matches the id. | |
| const route = SURFACE_ROUTES.find((entry) => { | |
| const basename = entry.path.slice(entry.path.lastIndexOf('/') + 1); | |
| return ( | |
| basename === item.id || | |
| basename === item.kind || | |
| entry.surfaceId === `console-${item.id}` || | |
| entry.surfaceId === `console-${item.kind}` | |
| ); | |
| }); |
🤖 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/block-surface-targets.ts` around lines 63 - 70, The
fallback lookup in the SURFACE_ROUTES find predicate must compare the basename
of each route path with item.id or item.kind, not only the complete path. Derive
the final path segment from entry.path and use it alongside the existing
surfaceId checks, preserving the current matching behavior.
| </h3> | ||
| <div className="flex flex-wrap gap-2"> | ||
| {(object.conformances ?? []).length === 0 ? ( | ||
| <span className="text-sm text-ij-ink-info">None yet — map on the Facets screen.</span> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Em dashes appear in four new console strings and comments. The console punctuation guideline forbids em dashes and en dashes in code comments, UI strings, and markdown. Replace each with a colon, period, comma, semicolon, or parentheses.
apps/console/src/views/model/settings/ModelSettingsView.tsx#L432-L432: change "None yet — map on the Facets screen." to "None yet. Map on the Facets screen."apps/console/src/views/model/settings/ModelSettingsView.tsx#L3-L6: change "no Twenty analog — Card + mapping rows" to "no Twenty analog: Card + mapping rows".apps/console/src/views/model/settings/FieldSettingsPanels.tsx#L441-L444: change "Pick local fields explicitly — auto-suggestions are only a starting point." to "Pick local fields explicitly. Auto-suggestions are only a starting point."apps/console/src/views/registry.tsx#L808-L809: change "P3 plan lens only — not a second computation authoring DAG" to "P3 plan lens only, not a second computation authoring DAG".
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."
📍 Affects 3 files
apps/console/src/views/model/settings/ModelSettingsView.tsx#L432-L432(this comment)apps/console/src/views/model/settings/ModelSettingsView.tsx#L3-L6apps/console/src/views/model/settings/FieldSettingsPanels.tsx#L441-L444apps/console/src/views/registry.tsx#L808-L809
🤖 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/model/settings/ModelSettingsView.tsx` at line 432,
Replace all four em dashes with the specified punctuation: update
ModelSettingsView.tsx lines 432 and 3-6, FieldSettingsPanels.tsx lines 441-444,
and registry.tsx lines 808-809. Preserve the surrounding text while using
periods, a colon, and a comma as specified.
Source: Coding guidelines
|
|
||
| vi.mock('server-only', () => ({})); | ||
|
|
||
| // Client helpers under test — fetch is stubbed. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the em dash from this code comment.
Console TypeScript comments must not contain em dashes or en dashes. Replace it with a period or colon.
🤖 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/program/programClient.gallery.test.ts` at line 5,
Update the comment above the client helper tests to replace the em dash with a
period or colon, preserving the comment’s meaning and avoiding em or en dashes.
Source: Coding guidelines
| /** Pre-run validate (ARD D24). Uses programmable_graph action `validate`. */ | ||
| export async function validateProgramDefinition( | ||
| definition: ProgramDefinition, | ||
| ): Promise<ProgramValidationResult> { | ||
| const response = await fetch('/api/harness/programmable-graph', { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify({ | ||
| tool: 'programmable_graph', | ||
| action: 'validate', | ||
| args: { definition, program: definition }, | ||
| }), | ||
| }); | ||
| const payload = (await response.json().catch(() => ({}))) as Record<string, unknown>; | ||
| if (!response.ok) { | ||
| return { | ||
| ok: false, | ||
| code: 'programmable_graph_http', | ||
| message: typeof payload.error === 'string' ? payload.error : `validate_${response.status}`, | ||
| nodeIds: [], | ||
| }; | ||
| } | ||
| if (payload.ok === false) { | ||
| const refusal = payload.refusal as | ||
| | { message?: string; code?: string; node_ids?: unknown } | ||
| | undefined; | ||
| const nodeIds = Array.isArray(refusal?.node_ids) | ||
| ? refusal!.node_ids!.map(String) | ||
| : []; | ||
| return { | ||
| ok: false, | ||
| code: refusal?.code ?? 'validate_refused', | ||
| message: refusal?.message ?? 'Program validation refused', | ||
| nodeIds, | ||
| }; | ||
| } | ||
| const nested = (payload.result && typeof payload.result === 'object' && !Array.isArray(payload.result) | ||
| ? payload.result | ||
| : payload) as Record<string, unknown>; | ||
| const checks = Array.isArray(nested.checks) | ||
| ? nested.checks.flatMap((item) => { | ||
| if (!item || typeof item !== 'object' || Array.isArray(item)) return []; | ||
| const row = item as Record<string, unknown>; | ||
| if (typeof row.requirement !== 'string') return []; | ||
| return [{ requirement: row.requirement, passed: row.passed !== false }]; | ||
| }) | ||
| : []; | ||
| return { | ||
| ok: true, | ||
| receipt: { | ||
| program_id: typeof nested.program_id === 'string' ? nested.program_id : '', | ||
| checks, | ||
| }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fail closed when the validation receipt has no checks array.
Lines 460-474 convert a successful but malformed response, such as {}, into ok: true with zero checks. ProgramView.runProgram then executes the program because it sees no failed checks.
Return ok: false when nested.checks is not an array. Keep an explicit empty array valid if the substrate uses it for a valid program.
🤖 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/program/programClient.ts` around lines 421 - 475,
Update validateProgramDefinition so a successful validation response returns ok:
false when nested.checks is not an array, with an appropriate validation-failure
code/message and no receipt. Preserve the existing successful path for an
explicit empty checks array and for valid check entries.
| const [validationSummary, setValidationSummary] = useState<string | null>(null); | ||
| const [validationNodeIds, setValidationNodeIds] = useState<string[]>([]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reset validation state when the active program changes.
openProgram, openStarter, forkProgram, and proposal flows call resetRunState, but that function does not clear these new values. The Run rail can show a validation receipt and node IDs from the previously active program.
Clear validationSummary and validationNodeIds in resetRunState.
🤖 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/program/ProgramView.tsx` around lines 291 - 292,
Update resetRunState to clear both validationSummary and validationNodeIds
whenever the active program changes, alongside its existing run-state resets.
Preserve the existing reset behavior for openProgram, openStarter, forkProgram,
and proposal flows.
| const raw = field.settings.raw ?? {}; | ||
| const canonical = raw.fieldType; | ||
| let token = field.type; | ||
| let variants = field.options.map((option) => option.value); | ||
| let vectorDim = 0; | ||
| let relationTarget = '*'; | ||
| let relationCardinality: 'one' | 'many' = 'many'; | ||
| if (canonical && typeof canonical === 'object' && !Array.isArray(canonical)) { | ||
| const typed = canonical as FieldType; | ||
| token = fieldTypeToTwentyToken(typed); | ||
| if (typed.kind === 'enum') variants = [...typed.variants]; | ||
| if (typed.kind === 'vector') { | ||
| token = 'RAW_JSON'; | ||
| vectorDim = typed.dim; | ||
| } | ||
| if (typed.kind === 'relation') { | ||
| relationTarget = typed.targetObjectTypeId; | ||
| relationCardinality = typed.cardinality; | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not overwrite the wire token with the lossy canonical token.
Line 149 replaces field.type with fieldTypeToTwentyToken(typed). The canonical mapping is lossy for compound tokens. A MULTI_SELECT, ARRAY, EMAILS, PHONES, LINKS, CURRENCY, RATING, NUMERIC, or RICH_TEXT field whose settings.raw.fieldType is present resolves to RAW_JSON, TEXT, or NUMBER.
The editor writes raw.fieldType on every save (ModelSettingsView.tsx lines 158-162). After the first save, reopening the field shows a different type in the select control, and the next save persists that changed token. This silently rewrites field types.
Prefer the wire token when it already agrees with the canonical kind. Use the canonical token only when the wire token disagrees.
🐛 Proposed fix
if (canonical && typeof canonical === 'object' && !Array.isArray(canonical)) {
const typed = canonical as FieldType;
- token = fieldTypeToTwentyToken(typed);
+ // Keep the wire token when it already round-trips to the same canonical
+ // kind, so compound tokens (MULTI_SELECT, EMAILS, CURRENCY) survive.
+ token =
+ twentyTokenToFieldType(field.type).kind === typed.kind
+ ? field.type
+ : fieldTypeToTwentyToken(typed);
if (typed.kind === 'enum') variants = [...typed.variants];📝 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 raw = field.settings.raw ?? {}; | |
| const canonical = raw.fieldType; | |
| let token = field.type; | |
| let variants = field.options.map((option) => option.value); | |
| let vectorDim = 0; | |
| let relationTarget = '*'; | |
| let relationCardinality: 'one' | 'many' = 'many'; | |
| if (canonical && typeof canonical === 'object' && !Array.isArray(canonical)) { | |
| const typed = canonical as FieldType; | |
| token = fieldTypeToTwentyToken(typed); | |
| if (typed.kind === 'enum') variants = [...typed.variants]; | |
| if (typed.kind === 'vector') { | |
| token = 'RAW_JSON'; | |
| vectorDim = typed.dim; | |
| } | |
| if (typed.kind === 'relation') { | |
| relationTarget = typed.targetObjectTypeId; | |
| relationCardinality = typed.cardinality; | |
| } | |
| } | |
| const raw = field.settings.raw ?? {}; | |
| const canonical = raw.fieldType; | |
| let token = field.type; | |
| let variants = field.options.map((option) => option.value); | |
| let vectorDim = 0; | |
| let relationTarget = '*'; | |
| let relationCardinality: 'one' | 'many' = 'many'; | |
| if (canonical && typeof canonical === 'object' && !Array.isArray(canonical)) { | |
| const typed = canonical as FieldType; | |
| // Keep the wire token when it already round-trips to the same canonical | |
| // kind, so compound tokens (MULTI_SELECT, EMAILS, CURRENCY) survive. | |
| token = | |
| twentyTokenToFieldType(field.type).kind === typed.kind | |
| ? field.type | |
| : fieldTypeToTwentyToken(typed); | |
| if (typed.kind === 'enum') variants = [...typed.variants]; | |
| if (typed.kind === 'vector') { | |
| token = 'RAW_JSON'; | |
| vectorDim = typed.dim; | |
| } | |
| if (typed.kind === 'relation') { | |
| relationTarget = typed.targetObjectTypeId; | |
| relationCardinality = typed.cardinality; | |
| } | |
| } |
🤖 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/data-model-contracts/src/twenty-metadata.ts` around lines 140 - 159,
In the field-type normalization block, preserve the original wire token in
`token` when it agrees with the canonical `FieldType` kind, and only use
`fieldTypeToTwentyToken(typed)` when `field.type` disagrees with that kind.
Update the logic around `canonical`, `typed`, and `token` so compound wire
tokens such as MULTI_SELECT, ARRAY, EMAILS, PHONES, LINKS, CURRENCY, RATING,
NUMERIC, and RICH_TEXT are not rewritten after saving, while existing enum,
vector, and relation metadata handling remains intact.
![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: 6c164f31f8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Loading observed model. | ||
| </div> | ||
| ) : ( | ||
| <DiagramLens {...lensProps} /> |
There was a problem hiding this comment.
The new lensProps adds import, export, declare, spawn, compare, and restore callbacks, but DiagramLens accepts the unchanged LensProps, and ForkDiagramCanvas neither accepts nor renders any of them. Because this diff simultaneously removes the old toolbar, proposal composer, lens tabs, and ModelInspector, users can no longer reach OKF import/export, schema proposals, field editing, or version comparison/restoration from the Models surface. Forward these actions into an actual canvas toolbar/inspector before removing the existing controls.
AGENTS.md reference: apps/console/AGENTS.md:L107-L107
Useful? React with 👍 / 👎.
| if (canonical && typeof canonical === 'object' && !Array.isArray(canonical)) { | ||
| const typed = canonical as FieldType; | ||
| token = fieldTypeToTwentyToken(typed); | ||
| if (typed.kind === 'enum') variants = [...typed.variants]; |
There was a problem hiding this comment.
When a field has settings.raw.fieldType, this overwrites its authoritative Twenty field.type with the reverse canonical mapping. That mapping is intentionally lossy, so saving an unchanged EMAILS, CURRENCY, RATING, MULTI_SELECT, or similar field immediately reloads it as TEXT, NUMBER, or RAW_JSON; a subsequent save then sends that replacement token and can change the schema type. Keep field.type unless the canonical payload represents a distinct tagged type that cannot be expressed by the presentation token.
AGENTS.md reference: apps/console/AGENTS.md:L108-L108
Useful? React with 👍 / 👎.
| } catch { | ||
| return handleLocalDevMetadata(method, segments, bodyText); | ||
| } |
There was a problem hiding this comment.
If the configured metadata service has a transient network failure, this silently handles the request in LocalDevMetadataStore, whose process-global state is neither tenant-keyed nor durable. In a multi-tenant production instance, users can therefore receive another tenant's fallback edits, and successful PATCH/POST responses can disappear on restart or when the upstream recovers. Restrict this fallback to an explicit development mode; production mutations should return the upstream failure.
AGENTS.md reference: apps/console/AGENTS.md:L186-L186
Useful? React with 👍 / 👎.
| const row = item as Record<string, unknown>; | ||
| if (typeof row.requirement !== 'string') return []; | ||
| return [{ requirement: row.requirement, passed: row.passed !== false }]; | ||
| }) | ||
| : []; |
There was a problem hiding this comment.
If the validate endpoint returns HTTP 200 with a missing, malformed, or differently nested checks value, this converts it to an empty array and then returns ok: true. runProgram consequently treats the response as zero failed checks and executes the program, bypassing the newly added validate-before-run gate during an upstream contract mismatch or malformed response. Require a structurally valid validation receipt, including explicit boolean outcomes, and fail closed otherwise.
Useful? React with 👍 / 👎.
| return mapped.length > 0 ? mapped : LOCAL_DEV_COMMAND_GALLERY; | ||
| } catch { | ||
| return LOCAL_DEV_COMMAND_GALLERY; |
There was a problem hiding this comment.
When gallery is empty, refused, or unreachable, this returns synthetic monitor entries, but selecting any of them still calls the same live gallery_fork endpoint with a fabricated program:* parent ID. In precisely the fallback scenario these rows cover, the fork therefore either cannot reach the service or names a template that the substrate did not return, so the advertised primary action only produces an error. Mark stand-in rows non-forkable or implement a matching local fork path instead of presenting them as actionable templates.
Useful? React with 👍 / 👎.
| export function resolveBlockSurfaceTarget( | ||
| item: Pick<BlockPaletteItem, 'descriptorId' | 'kind' | 'id'>, | ||
| ): BlockSurfaceTarget | null { | ||
| const byDescriptor = BY_DESCRIPTOR_ID[item.descriptorId]; |
There was a problem hiding this comment.
This resolver is only imported by its unit test; no production component calls it. The unchanged IntuiShell.handleAddBlock path still sends every Blocks palette selection to placeBlockInEditor, which creates a new view instance in the current editor, so Records, Models, Commands, and the other mapped items never navigate to the page-owned surfaces described here. Invoke this resolver from the palette handler and navigate/activate its returned surface before falling back to embedded placement.
Useful? React with 👍 / 👎.
| // Soft refresh: keep the current canvas mounted while re-reading. A hard | ||
| // loading flash during drag/registry polls is what made nodes "vanish". | ||
| if (!hasPaintedModelRef.current) setLoading(true); |
There was a problem hiding this comment.
The soft-refresh guard is also used when topicId changes, so if the previous topic had any painted nodes, the old observed and declared model remains interactive until the new fetch completes. During that window, actions such as a ghost card's Declare callback use the new topicId with an observed key from the previous topic, potentially declaring the wrong schema in the newly selected scope. Preserve the canvas only for same-topic registry refreshes; a scope change must clear or disable the prior model while loading.
Useful? React with 👍 / 👎.
| usage: | ||
| 'P3 plan lens only — not a second computation authoring DAG; author computation on Program canvas', | ||
| placements: ['ground', 'full'], |
There was a problem hiding this comment.
This change only renames the descriptor and describes it as non-authoring while continuing to render the unchanged GoalStackView. That component still exposes the capability palette, accepts capability drops through queue_affordance, enables nodesConnectable, and persists new DEPENDS_ON links, so /goals remains a second computation-authoring DAG despite the new label. Remove or disable those mutation affordances when mounting the plan lens, or fold them into the Program canvas as intended.
Useful? React with 👍 / 👎.
Summary
gallery/gallery_fork), Goal Stack demoted to plan lens, records validation refusal UX, and LocalDev metadata REST stand-in with smoke script.Companion substrate PR: https://github.com/Travis-Gilbert/Theorem/pull/511
Test plan
npm --prefix packages/data-model-contracts test(twenty-metadata)programClient.gallery,local-dev-metadata-store,block-surface-targets, view-routing retirementnpm --prefix apps/console run gate:twentynode apps/console/scripts/smoke-metadata-rest.mjswhen metadata URL is set/Data-model/settings,/commands, program Run blocked until validate passesSummary by CodeRabbit