Skip to content

Ontology Studio: suggest a property per column, draw bound columns, and pick mappings by label - #37215

Merged
harshach merged 32 commits into
mainfrom
harshach/ontology-studio/v7-parity-gaps
Nov 6, 2026
Merged

harshach merged 32 commits into
mainfrom
harshach/ontology-studio/v7-parity-gaps

Conversation

@harshach

@harshach harshach commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator

Describe your changes:

No linked issue: this follows up #36868, and the maintainer chose not to open one.

After #36868 merged, a read-only audit checked each item of the v7 completion plan against the code. 133 of 139 were built as planned and 3 were changed on purpose. Four fell short, and two time limits had never been measured. This PR closes the four gaps and fixes what review of those changes found.

  • Binding a table suggests a property for each column. Linking a table, or pressing Bind columns on a table already linked, lists its columns. Each column has a checkbox and a property select, pre-filled from its name and marked Suggested. The user can change or uncheck rows, or skip the step. Saving keeps every binding the step does not show: a column's second property, columns past the 1000-column cap, and every binding on a table whose columns failed to load.
  • The Data canvas draws bound columns as nodes. Opening a table's column toggle draws each bound column as a node joined to the table by an edge, with up to 5 per table, then Show N more. Selecting one, by click or by keyboard, opens its binding: column, property and table. On a 1280-pixel window, the canvas pans so the side panel never covers the selected column or the columns beside it.
  • The new concept drawer picks mappings by label. The drawer uses the edit form's picker, which searches concepts across ontologies and accepts a pasted IRI. Enter adds a pasted IRI. While other ontologies' concepts load, the list shows a loading row and then the matches, with no retyping. The row says "Enter a valid concept IRI." only after the user tries to add text that is neither a concept nor an IRI, by Enter, Add mapping, leaving the field or Create. Create never drops that text. A mapping that is already added says so.
  • Disabled controls say why. In All ontologies, the disabled Publish shows its reason as text. In glossary scope, Add concept says to finish or close the open concept first.

Type of change:

  • Improvement

High-level design:

  • OntologyColumnMappingStep is the binding step. suggestPropertyForColumn shares its scoring with suggestColumnForProperty. A save writes one revision-checked replaceAsset per table, built from the table's existing bindings. The prototype has the same step behind if (false) and instead binds each picked column to its guess with no review. This PR keeps the reviewable step, so nothing is bound without being seen. ADR:2026-11-05-binding-a-tables-columns-shows-each-suggestion-and-keeps-unseen-bindings records both rules and the 5-node cap.
  • utils/dataColumnLayout.ts lays out the column nodes, and OntologyDataColumnPanel shows a selected column. utils/dataPanelView.ts computes the pan that keeps a selected table and its columns left of an open panel. The canvas has no native scroll container, so it pans instead of reserving space.
  • useMappingTargetOptions gives the drawer and the edit form one option source. OntologyIriInput gains onSubmit (Enter on a pasted IRI) and isLoading (a loading row).
  • UI only. No schema, REST or server change.

Tests:

Use cases covered

  • Link a table, see a suggested property for each matching column, uncheck one, save. The API and the table page show only the checked columns.
  • Skip the step. The table stays linked with no column bindings.
  • Bind columns on a table already linked, and change one suggestion.
  • Save the step without losing a column's second binding, a binding on an unlisted column, or the bindings of a table whose columns failed to load.
  • Open a table's columns in Data mode. Select a column node by click and by keyboard, and read its binding. A 7-column table shows 5 nodes and Show 2 more.
  • At 1280 by 800, select one column and click the next while the panel stays open.
  • Create a concept with a mapping picked by label from another ontology and one pasted IRI, then add a pasted IRI with Enter.
  • Type a partial label and see no error. Press Add mapping, Enter or Create on text that is neither a concept nor an IRI, and see the error with the text kept. Type an added IRI again and see that it is already added.
  • See why Publish is disabled in All ontologies, and why Add concept is disabled while a glossary concept draft is open.

Unit tests

  • Added: OntologyColumnMappingStep.test.tsx (12), OntologyBindingSuggestion.utils.test.ts, OntologyStudioModel.utils.test.ts, dataColumnLayout.test.ts (6), dataPanelView.test.ts (4), OntologyDataGraph.test.tsx, OntologyNativeDataMode.test.tsx, OntologyNewClassPanel.test.tsx, OntologyExplorerPage.test.tsx, OntologyConceptEditor.test.tsx.

Backend integration tests

  • Not applicable (no backend change).

Playwright (UI) tests

  • Added or updated, each run against a local server, with new cases run twice:
    • OntologyNativeDataMode.spec.ts: binding step (4 cases), 15 of 15.
    • OntologyNativeDataColumns.spec.ts (new): column nodes, the cap, and the 1280 cases in both modes. With OntologyNativeDataMode.spec.ts, 22 of 22.
    • OntologyConceptDrawer.spec.ts: mapping by label, pasted IRI, Enter, tabbing into the mapping field, and the error shown only on commit. 28 of 28.
    • OntologyReview.spec.ts 27 of 27, and OntologyStudio.spec.ts 8 of 8.

Measurements

  • A 400-concept graph draws its nodes in 1208 ms median and 1339 ms worst over 5 runs, on the production bundle. The plan's limit is 3 s.
  • On a fresh server, RdfFirstIndexRunIT (rdf lane) queued the first RDF index run and saw it complete about 13 s later. The plan's limit is one minute. The database held seed data only.

Manual testing performed

None beyond the runs above.

UI screen recording:

Not attached yet.

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • My PR is linked to a GitHub issue via Fixes #<issue-number> above.
  • I have commented on my code, particularly in hard-to-understand areas.
  • For JSON Schema changes: not applicable.
  • For a UI feature or feature task: recording not attached yet.
  • I have added tests (unit / integration / Playwright as applicable) and listed them above.
  • Self-review: I reviewed the final diff against REVIEW.md.
  • Decision check: ADR:2026-11-05-binding-a-tables-columns-shows-each-suggestion-and-keeps-unseen-bindings.

@harshach
harshach requested a review from a team as a code owner November 6, 2026 05:04
@harshach
harshach requested review from anuj-kumary and a balanced review from Copilot November 6, 2026 05:04
@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

❌ PR checklist incomplete

This PR cannot be merged until the following are addressed on its linked issue:

  • No GitHub issue is linked. Link an issue in the Development section of the PR (or add Fixes #12345 to the description). For a same-org cross-repo issue, add Fixes open-metadata/<repo>#123 to the description.

The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically.

Maintainers can bypass this check by adding the skip-pr-checks label.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@github-actions github-actions Bot added the safe to test Add this label to run secure Github workflows on PRs label Nov 6, 2026
@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

🔄 Playwright impact map auto-refreshed

This PR touched specs or UI source that changed the source→spec routing map. I regenerated .github/playwright/impact-map.generated.json and pushed the diff to this branch.

- source entries: 1063 → 1065
- 2 added, 0 removed, 15 changed spec-list

New source→spec entries:
  openmetadata-ui/src/main/resources/ui/src/components/governance/ontology/OntologyConceptInspector/OntologyColumnMappingStep.tsx
  openmetadata-ui/src/main/resources/ui/src/components/governance/ontology/OntologyGraphView/OntologyDataColumnPanel.tsx

Entries whose spec list changed:
  openmetadata-ui/src/main/resources/ui/playwright/e2e/Utils/appMode.ts
  openmetadata-ui/src/main/resources/ui/playwright/support/entity/TableClass.ts
  openmetadata-ui/src/main/resources/ui/playwright/support/fixtures/isolatedUser.ts
  openmetadata-ui/src/main/resources/ui/playwright/utils/apiResponse.ts
  openmetadata-ui/src/main/resources/ui/playwright/utils/common.ts
  openmetadata-ui/src/main/resources/ui/playwright/utils/ontologyStudio.ts
  openmetadata-ui/src/main/resources/ui/src/components/OntologyExplorer/OntologyDataGraph.tsx
  openmetadata-ui/src/main/resources/ui/src/components/governance/ontology/OntologyGraphView/OntologyDataAssetPanel.tsx
  openmetadata-ui/src/main/resources/ui/src/components/governance/ontology/OntologyGraphView/OntologyNativeDataMode.tsx
  openmetadata-ui/src/main/resources/ui/src/generated/entity/data/glossaryTerm.ts
  … and 5 more

What is this file? It is the auto-generated half of Playwright's PR planner. It routes "if source X changes, run specs Y" by walking spec imports and cross-referencing getByTestId strings. Hand-authored routing in impact-map.json always wins on conflict.

What if I want to regenerate locally instead? Run this before pushing your next change to skip the bot commit:

python3 .github/scripts/generate_playwright_impact_map.py
git add .github/playwright/impact-map.generated.json
git commit --amend --no-edit  # or a separate commit

Copilot AI balanced review requested due to automatic review settings November 6, 2026 05:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@github-actions

github-actions Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

⚠️ UI Checkstyle passed — lint findings in changed files

🔍 ESLint findings in this PR's files — 0 error(s), 3 warning(s)

Errors block the build. Warnings do not yet — they are rules whose backlog is still
being worked down, listed so this PR does not add to it. See docs/ui-code-quality-gate.md.

0 error(s), 3 warning(s) across 3 changed file(s).

Count Rule
2 openmetadata-imports/no-api-calls-in-iteration
1 openmetadata-imports/review-sequential-api-calls
All findings
Location Rule Message
🟡 src/components/governance/ontology/OntologyConceptEditor/useReadableConceptOptions.ts:48:32 openmetadata-imports/no-api-calls-in-iteration Avoid issuing one API request per item. Fetch at the data owner, use a bulk endpoint, or use useQueries with an intentional concurrency policy.
🟡 src/components/governance/ontology/OntologyGraphView/OntologyGraphView.tsx:805:9 openmetadata-imports/review-sequential-api-calls Review these sequential API requests. If they are independent, start them together with Promise.all/Promise.allSettled; keep sequencing only when data-dependent
🟡 src/components/governance/ontology/OntologyNewClassPanel/OntologyNewClassPanel.tsx:479:19 openmetadata-imports/no-api-calls-in-iteration Avoid issuing one API request per item. Fetch at the data owner, use a bulk endpoint, or use useQueries with an intentional concurrency policy.

Fix locally (fast - only checks files changed in this branch):

make ui-checkstyle-changed

@github-actions

github-actions Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 81%
81.63% (141727/173603) 69.05% (51867/75111) 72.04% (29061/40338)

Copilot AI balanced review requested due to automatic review settings November 6, 2026 05:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

…dio/v7-parity-gaps

# Conflicts:
#	openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/OntologyReview.spec.ts
Copilot AI balanced review requested due to automatic review settings November 6, 2026 05:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

Copilot AI balanced review requested due to automatic review settings November 6, 2026 06:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@gitar-bot

gitar-bot Bot commented Nov 6, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 3 closed / 3 findings

🟡 Medium risk · Reviewable column suggestions now save ontology bindings per table; an incorrect selection could misrepresent table data in the ontology.

Adds per-column property suggestions when binding a table, draws bound columns as selectable canvas nodes, and lets mappings be picked by label in the concept drawer. Resolved the multi-table save that stopped after the first table while reporting success, Enter not adding a label-picked mapping, and the untranslated partial-save message in 20 non-English locales. No issues found.

✅ 3 closed
✅ Bug: Multi-table save stops after first table yet reports success

📄 openmetadata-ui/src/main/resources/ui/src/components/governance/ontology/OntologyConceptInspector/OntologyColumnMappingStep.tsx:190-204
In save(), when a replaceAsset is confirmed but its re-read comes back unavailable, the loop shows the success toast, calls onClose() and returns. Any tables after that one are never written. The asset picker can link several tables in one go (linked.filter(asset => asset.type === TABLE)), so the user reviews rows for every table, sees a 'saved' message, and the step closes with only the first table bound. A table's reviewed bindings are dropped without any warning.

Fix: stopping is still right, because there is no fresh revision to continue with. But the step should stay open, or name the tables it did not write, instead of closing with a success toast.

✅ Bug: Enter does not add a mapping that was picked by its label

📄 openmetadata-ui/src/main/resources/ui/src/components/governance/ontology/OntologyIriInput/OntologyIriInput.tsx:87-101
submitOnEnter checks the input's visible text (event.currentTarget.value) with isAbsoluteOntologyIri. When the user picks a target by label, or types a label that matches exactly one option, the input still shows the label while value holds the IRI. So !isAbsoluteOntologyIri(text) returns early and onSubmit (addMapping) never runs. In the new concept drawer, Enter does nothing for label-picked mappings, though it works for pasted IRIs. The user has to press Add mapping instead.

Fix: decide from the committed value, or from selected, rather than from the displayed text.

✅ Quality: New partial-save message left in English in all 20 non-English locales

📄 openmetadata-ui/src/main/resources/ui/src/locale/languages/ar-sa.json:5762 📄 openmetadata-ui/src/main/resources/ui/src/locale/languages/de-de.json 📄 openmetadata-ui/src/main/resources/ui/src/locale/languages/fr-fr.json 📄 openmetadata-ui/src/main/resources/ui/src/locale/languages/ja-jp.json 📄 openmetadata-ui/src/main/resources/ui/src/locale/languages/zh-cn.json
message.ontology-column-binding-partial was synced by yarn i18n and kept the English text verbatim in every non-en-us catalog: ar-sa, de-de, es-es, fr-fr, gl-es, he-he, it-it, ja-jp, ko-kr, mr-in, nl-nl, pr-pr, pt-br, pt-pt, ru-ru, sv-se, th-th, tr-tr, zh-cn and zh-tw. Users in those locales get the English alert ('Saved … Not saved: …') when a multi-table column binding save stops partway. The i18n rule calls English under a non-English key a reviewable defect, and this is not a deliberate case like a product name. Translate the string in each locale and keep the {{saved}} and {{tables}} placeholders.

Verification

Rules No rules evaluated

Functional validation Not enabled

Set up

Auto-approval Not enabled

Set up

For more verification details, comment gitar display:verbose.

Options

Display: compact → Showing compact verification results.

Comment with these commands to change the behavior for this request:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source

@harshach
harshach merged commit 37276be into main Nov 6, 2026
67 of 70 checks passed
@harshach
harshach deleted the harshach/ontology-studio/v7-parity-gaps branch November 6, 2026 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend safe to test Add this label to run secure Github workflows on PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants