Skip to content

docs(ui): define UI folder structure + file naming, wire into agent config - #31812

Merged
chirag-madlani merged 2 commits into
mainfrom
ui-folder-structure-handbook
Aug 22, 2026
Merged

docs(ui): define UI folder structure + file naming, wire into agent config#31812
chirag-madlani merged 2 commits into
mainfrom
ui-folder-structure-handbook

Conversation

@chirag-madlani

@chirag-madlani chirag-madlani commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Describe your changes:

I rewrote openmetadata-ui/src/main/resources/ui/DEVELOPER_HANDBOOK.md to define a
domain/feature folder structure and a single file-naming scheme, because the UI tree has grown
to 5,305 files with 77 top-level component folders (667 in common/ alone) and 526 flat
utils/ entries, with no documented grouping rule — and naming has forked in the process (830
bare .tsx against 456 .component.tsx, 411 .interface.ts against 4 .types.ts). I then
wired the handbook into every agent-facing entry point, since it was previously referenced only
from the UI README.md while five AI-facing docs still mandated the legacy naming it supersedes.
No source files change — this is documentation and agent configuration only.

Type of change:

  • Documentation

High-level design:

Layer-first, domain-grouped. components/, pages/, rest/, utils/, hooks/,
constants/ and interface/ stay as top-level layers; domain and feature grouping goes inside
each one, so governance/glossary is a single coordinate that resolves in every layer:

components/governance/glossary/GlossaryList/
pages/governance/glossary/GlossaryListPage/
rest/governance/glossary.api.ts
utils/governance/glossary.utils.ts

Five domains (discovery, governance, observability, insights, platform), with
cross-cutting features (lineage, data-contract, entity, activity-feed) at the domain level
under one stated test: if two domains would both claim a feature, it has no domain. That rule
exists so adding a folder doesn't reopen a taxonomy argument on every PR.

Alternative rejected — feature-first (src/domains/<domain>/<feature>/ owning its own
components, pages and api). It colocates a feature better, but eslint-rules/openmetadata-imports.mjs
hardcodes SOURCE_LAYERS (lines 18-29) and getLayer() (lines 71-83) resolves a file's layer from
the first path segment after src/. A new top-level root falls outside that set, silently no-opping
all ten custom import rules — no-cross-page-imports, no-rest-ui-imports, no-hook-ui-imports,
no-impure-pure-utils, no-internal-barrel-imports and the rest — for every new feature. Keeping
layers at the top keeps those rules live. The cost, stated in the handbook: a feature spans several trees.

Agent config (2nd commit). The handbook is useless to an agent that never reads it, and five
docs actively contradicted it. Each entry point now points at the handbook and carries the same
naming table: .claude/rules/frontend-react.md (auto-loads on UI *.{ts,tsx}), CLAUDE.md
(always loaded; AGENTS.md symlinks it), docs/index.md, skills/openmetadata-workflow/SKILL.md,
skills/agents/frontend-reviewer.md, .github/copilot-instructions.md and
openmetadata-ui/UI_PR_REVIEW_GUIDELINES.md. Every one states that legacy
.component.tsx/.interface.ts files must not be renamed, so reviewers don't generate rename
churn across 5,305 existing files.

Three corrections to existing guidance, verified against the tree:

  • src/routes/ and src/store/ were documented but do not exist. Routers live in
    components/AppRouter/ (GlossaryRouter, DomainRouter, SettingsRouter); Zustand stores live
    in src/hooks/ (useApplicationStore, useSearchStore, useDomainStore).
  • index.ts barrels were recommended in two places despite no-internal-barrel-imports reporting
    them and frontend-performance.md explaining why.
  • The suggested assets/icons/index.ts icon re-export is the exact pathological case that rule exists to prevent.

Known follow-up, flagged in §6 rather than silently assumed: the absolute-import convention does
not work in application code yet. jest.config.js:112 sets moduleDirectories: ['node_modules', 'src']
so it resolves under Jest, but tsconfig.json has no baseUrl and vite.config.ts aliases only @.
That is why exactly one file in the repo uses a bare-layer import and it is a .test.tsx. Enabling it
needs "baseUrl": "./src" plus matching Vite resolve.alias entries — deliberately left out to keep
this PR documentation-only.

Tests:

Use cases covered

Not applicable — documentation and agent config only, no runtime code changed.

Unit tests

Not applicable — no logic changed.

Backend integration tests

  • Not applicable (no backend API changes).

Ingestion integration tests

  • Not applicable (no ingestion changes).

Playwright (UI) tests

  • Not applicable (no UI behaviour changes — markdown only).

Manual testing performed

  1. Verified every path the handbook references resolves: docs/formutils.md,
    playwright/PLAYWRIGHT_DEVELOPER_HANDBOOK.md, root CLAUDE.md,
    components/AppRouter/withSuspenseFallback.tsx.
  2. Confirmed the stale references are genuinely absent — src/routes/, src/store/, src/stores/
    do not exist; src/context/ (11 providers) is canonical over the stray src/contexts/ (1 file),
    which the handbook now notes.
  3. Confirmed every yarn command cited exists in package.json: ui-checkstyle:changed (line 65),
    lint (36), organize-imports:cli (52), license-header-fix (30).
  4. Checked all five domain names against the 77 existing components/ folders case-insensitively —
    no collisions on a case-insensitive filesystem.
  5. Grepped all AI-facing docs for the superseded naming mandate — ComponentName.component.tsx /
    ComponentName.interface.ts now returns zero hits outside the explicit "legacy, do not rename" notes.
  6. Ran python3 scripts/harness/check_harness.py — 5 warnings, all pre-existing on main and
    unrelated; no new dead references, and frontend-react.md is 94 lines against its 100-line budget.

UI screen recording / screenshots:

Not applicable — markdown documentation only, no rendered UI changes.

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation> — no issue linked, see note above.
  • 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, no schema changes.
  • For UI changes: not applicable, no rendered UI changes.
  • I have added tests (unit / integration / Playwright as applicable) and listed them above — not applicable, documentation only.

🤖 Generated with Claude Code

Greptile Summary

The PR establishes a layer-first, domain/feature-based organization and unified naming conventions for new UI code, then propagates that guidance across contributor and agent documentation.

  • Defines five UI domains and placement rules for cross-cutting features.
  • Standardizes role-based filenames while preserving legacy naming.
  • Documents import boundaries, routing composition, state placement, testing, and barrel restrictions.
  • Updates review guidelines and repository guidance to reference the handbook.

Confidence Score: 3/5

The PR does not appear safe to merge until the outstanding type-check command and route-constant placement guidance are corrected.

The handbook still bypasses the existing Yarn type-check script and directs route constants away from the shared registry consumed by routers, links, and breadcrumbs.

Files Needing Attention: openmetadata-ui/src/main/resources/ui/DEVELOPER_HANDBOOK.md, .claude/rules/frontend-react.md

Important Files Changed

Filename Overview
openmetadata-ui/src/main/resources/ui/DEVELOPER_HANDBOOK.md Rewrites the UI organization handbook, but the previously reported type-check and route-constant instructions remain unresolved.
.claude/rules/frontend-react.md Aligns agent-facing React guidance with the new folder and naming conventions while retaining the existing noncanonical type-check command.
openmetadata-ui/UI_PR_REVIEW_GUIDELINES.md Updates UI review criteria to apply the new placement and naming conventions.
CLAUDE.md Adds the developer handbook to the repository-wide UI guidance index.
.github/copilot-instructions.md Propagates the new structure and naming rules into Copilot review instructions.

Reviews (2): Last reviewed commit: "docs: wire the UI folder-structure handb..." | Re-trigger Greptile

…entions

The UI tree has grown to 5,305 files with 77 top-level component folders
(667 in common/ alone) and 526 flat utils, with no documented grouping
scheme. File naming has forked: 830 bare .tsx vs 456 .component.tsx,
411 .interface.ts vs 4 .types.ts.

Document a layer-first structure that keeps components/, pages/, rest/,
utils/, hooks/ as top-level layers and adds domain/feature grouping
inside each, so `governance/glossary` is one coordinate across layers.
Keeping layers at the top preserves the custom ESLint rules in
eslint-rules/, which resolve a file's layer from the first path segment
after src/.

Also correct three stale claims in the handbook:
- src/routes/ and src/store/ do not exist; routers live in
  components/AppRouter/ and Zustand stores in src/hooks/
- index.ts barrels were recommended in two places despite the
  no-internal-barrel-imports rule reporting them
- the assets/icons/index.ts icon barrel is the exact case that rule exists
  to prevent

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 20, 2026 06:57

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 reached their quota limit.

@github-actions

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.

@github-actions github-actions Bot added safe to test Add this label to run secure Github workflows on PRs UI UI specific issues labels Aug 20, 2026
Comment thread openmetadata-ui/src/main/resources/ui/DEVELOPER_HANDBOOK.md
Comment thread openmetadata-ui/src/main/resources/ui/DEVELOPER_HANDBOOK.md
Comment on lines +207 to +210
> them yet — `tsconfig.json` has no `baseUrl`, and `vite.config.ts` aliases only `@`. Before using
> absolute imports in application code, add `"baseUrl": "./src"` to `tsconfig.json` and matching
> entries to the Vite `resolve.alias` block. Until then, application code must keep using relative
> paths; only tests can use the absolute form.

@gitar-bot gitar-bot Bot Aug 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Quality: "vite.config.ts aliases only @" is inaccurate

The handbook states "vite.config.ts aliases only @", but vite.config.ts:218-238 defines several resolve.alias entries (lodash, process, Quill, @, ~antd, antd, @deuex-solutions/react-tour, luxon). The intended point — that @ is the only alias mapping a source path (src) and bare-layer imports won't resolve in Vite — still holds, but the literal claim is wrong. Consider rewording to "@ is the only alias mapping into src" to keep the doc precise.

Reword to reflect that Vite has other package aliases but only @ maps into src.:

> `tsconfig.json` has no `baseUrl`, and `@` is the only Vite `resolve.alias` entry that maps
> into `src` (`@` → `src`); no alias resolves bare layer paths like `components/…`.

Was this helpful? React with 👍 / 👎

@github-actions

github-actions Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

✅ Playwright Results — workflow succeeded

Validated commit 6c562c3aaaadfcab59796f3e646e8df7e678cb73 in Playwright run 32449182329, attempt 1.

✅ 553 passed · ❌ 0 failed · 🟡 0 flaky · ⏭️ 0 skipped · 🧰 0 lifecycle flaky

Performance

Blocking targets: ✅ met · Optimization targets: 🟡 in progress

Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting.

🕒 Full workflow signal wall (to summary) 50m 44s

⏱️ Max setup 4m 18s · max shard execution 16m 15s · max shard-job elapsed before upload 19m 31s · reporting 4s

🌐 215.50 requests/attempt · 2.83 app boots/UI scenario · 11.28% common-shard skew

Optimization targets still in progress:

  • Browser traffic was 215.5 requests per attempt (convergence target: fewer than 200).
  • Application boot ratio was 2.83 per UI scenario (1623 boots / 574 scenarios; convergence target: at most 1).
Shard Passed Failed Flaky Skipped Lifecycle failed Lifecycle flaky
✅ Shard chromium-01 103 0 0 0 0 0
✅ Shard chromium-02 102 0 0 0 0 0
✅ Shard chromium-03 102 0 0 0 0 0
✅ Shard chromium-04 92 0 0 0 0 0
✅ Shard data-asset-rules-01 61 0 0 0 0 0
✅ Shard domain-isolation-01 16 0 0 0 0 0
✅ Shard global-state-01 34 0 0 0 0 0
✅ Shard ingestion-01 1 0 0 0 0 0
✅ Shard reindex-01 2 0 0 0 0 0
✅ Shard search-01 11 0 0 0 0 0
✅ Shard search-rbac-01 29 0 0 0 0 0

📦 Download artifacts

How to debug locally
# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

DEVELOPER_HANDBOOK.md was referenced only from the UI README, so no
agent-facing config pointed at it. Worse, five AI-facing docs mandated
the legacy naming (`ComponentName.component.tsx` /
`ComponentName.interface.ts`) that the handbook supersedes for new code,
so an agent following the rules and an agent following the handbook
would produce different filenames.

Point every agent entry point at the handbook and reconcile the naming
guidance:
- .claude/rules/frontend-react.md — auto-loads on UI *.{ts,tsx}; now
  carries the domain/feature placement rule and the new naming table
- CLAUDE.md — added to "Repo coding conventions" (AGENTS.md symlinks it)
- docs/index.md — indexed in the UI reference table
- skills/openmetadata-workflow/SKILL.md — the UI component row now reads
  the handbook first
- skills/agents/frontend-reviewer.md — reviews placement and naming
- .github/copilot-instructions.md, openmetadata-ui/UI_PR_REVIEW_GUIDELINES.md

Every one states that legacy `.component.tsx`/`.interface.ts` files must
NOT be renamed, so reviewers don't generate rename churn across the 5,305
existing files.

check_harness.py reports the same 5 pre-existing warnings, no new ones;
frontend-react.md is 94 lines, under the 100-line rule budget.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 21, 2026 05:03

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 reached their quota limit.

@chirag-madlani chirag-madlani changed the title docs(ui): define domain/feature folder structure and file naming conventions docs(ui): define UI folder structure + file naming, and wire it into agent config Aug 21, 2026
@gitar-bot

gitar-bot Bot commented Aug 21, 2026

Copy link
Copy Markdown
Code Review 👍 Approved with suggestions 0 resolved / 1 findings

Defines a domain/feature folder structure and unified file naming conventions in the UI developer handbook to prevent tree divergence. Consider addressing the minor Vite configuration alias description in the text.

💡 Quality: "vite.config.ts aliases only @" is inaccurate

📄 openmetadata-ui/src/main/resources/ui/DEVELOPER_HANDBOOK.md:207-210

The handbook states "vite.config.ts aliases only @", but vite.config.ts:218-238 defines several resolve.alias entries (lodash, process, Quill, @, ~antd, antd, @deuex-solutions/react-tour, luxon). The intended point — that @ is the only alias mapping a source path (src) and bare-layer imports won't resolve in Vite — still holds, but the literal claim is wrong. Consider rewording to "@ is the only alias mapping into src" to keep the doc precise.

Reword to reflect that Vite has other package aliases but only `@` maps into src.
> `tsconfig.json` has no `baseUrl`, and `@` is the only Vite `resolve.alias` entry that maps
> into `src` (`@` → `src`); no alias resolves bare layer paths like `components/…`.
🤖 Prompt for agents
Code Review: Defines a domain/feature folder structure and unified file naming conventions in the UI developer handbook to prevent tree divergence. Consider addressing the minor Vite configuration alias description in the text.

1. 💡 Quality: "vite.config.ts aliases only `@`" is inaccurate
   Files: openmetadata-ui/src/main/resources/ui/DEVELOPER_HANDBOOK.md:207-210

   The handbook states "vite.config.ts aliases only `@`", but vite.config.ts:218-238 defines several `resolve.alias` entries (`lodash`, `process`, `Quill`, `@`, `~antd`, `antd`, `@deuex-solutions/react-tour`, `luxon`). The intended point — that `@` is the only alias mapping a source path (`src`) and bare-layer imports won't resolve in Vite — still holds, but the literal claim is wrong. Consider rewording to "`@` is the only alias mapping into `src`" to keep the doc precise.

   Fix (Reword to reflect that Vite has other package aliases but only `@` maps into src.):
   > `tsconfig.json` has no `baseUrl`, and `@` is the only Vite `resolve.alias` entry that maps
   > into `src` (`@` → `src`); no alias resolves bare layer paths like `components/…`.

Options

Display: compact → Showing less information.

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

@chirag-madlani
chirag-madlani added this pull request to the merge queue Aug 21, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 21, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🚦 Removed from the merge queue — failed_checks (2026-08-21T17:03:47Z)

Blocked the queue: playwright-summary

@chirag-madlani
chirag-madlani merged commit 4729d34 into main Aug 22, 2026
99 of 171 checks passed
@chirag-madlani
chirag-madlani deleted the ui-folder-structure-handbook branch August 22, 2026 04:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test Add this label to run secure Github workflows on PRs UI UI specific issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants