[refactor] Break the antd ColumnsType coupling with a local ColumnDef - #5959
Open
ardaerzin wants to merge 1 commit into
Open
[refactor] Break the antd ColumnsType coupling with a local ColumnDef#5959ardaerzin wants to merge 1 commit into
ardaerzin wants to merge 1 commit into
Conversation
…ColumnDef
Step 1 of the table port, pulled forward: mechanical, zero visual risk, and it
stops the type debt growing while every later table step stays blocked.
ColumnDef<T> lives in InfiniteVirtualTable and antd's ColumnsType becomes an
adapter applied once, at columns={toAntdColumns(finalColumns)}. The table's
rendering is untouched; <Table virtual> stays.
Function-valued column props are declared with method syntax, not property
syntax. Method params check bivariantly, so a column may still write
render: (date: string) => … and stay assignable. That is what antd bought with
any, obtained here without one; property syntax with unknown would have broken
about fifty call sites.
getObservabilityColumns also drops its last antd import, swapping Tag for the
SpanIdChip built in WP3.
Two things the plan had wrong. groupColumns.ts sits outside the directory the
measurement covered but is antd-typed and feeds buildEntityColumns, so it had
to move in the same change or the package would not compile. And the observability
types.d.ts antd import did not fall out for free: it also imported Avatar for an
exported AvatarTreeContentProps, which turned out to have no references anywhere
in oss, ee, packages or mobile, so it is deleted.
fromAntdColumns is exported but unused. It exists for the four raw <Table> call
sites deliberately left on antd.
dosubot
Bot
added
the
size:XL
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
dosubot
Bot
added
frontend
refactoring
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe change adds shared table column definition types, migrates virtual table and OSS column contracts from Ant Design types, and converts local columns to Ant Design columns at the rendering boundary. Observability span IDs now use ChangesShared table column type migration
Estimated code review effort: 3 (Moderate) | ~25 minutes Mergeability Score: 🔵 Low · up to The refactor remains mergeable with owner follow-up: grouped columns are currently exposed through a leaf-only return type, which can hide nested group headers from consumers, and fixed columns using "start"/"end" may behave inconsistently with the supported Ant Design 5.0 range. 🚥 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 |
Contributor
Railway Preview Environment
|
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 127b3489-f8e0-44a4-ac4c-3a136f885a17
📒 Files selected for processing (34)
web/oss/src/components/EvalRunDetails/etl/useEtlColumns.tsxweb/oss/src/components/EvalRunDetails/utils/buildPreviewColumns.tsxweb/oss/src/components/EvaluationRunsTablePOC/hooks/useEvaluationRunsColumns/index.tsxweb/oss/src/components/SharedDrawers/TraceDrawer/components/TraceContent/components/LinkedSpansTabItem/index.tsxweb/oss/src/components/TestcasesTableNew/components/TestcasesTableShell.tsxweb/oss/src/components/TestcasesTableNew/utils/groupColumns.tsweb/oss/src/components/TestsetsTable/assets/createTestsetsColumns.tsxweb/oss/src/components/TestsetsTable/hooks/useTestsetsColumns.tsxweb/oss/src/components/pages/agents/AgentsTableSection.tsxweb/oss/src/components/pages/observability/assets/getObservabilityColumns.tsxweb/oss/src/components/pages/observability/assets/types.d.tsweb/oss/src/components/pages/prompts/components/PromptsTableSection.tsxweb/packages/agenta-entity-ui/src/shared/EntityTable.tsxweb/packages/agenta-ui/src/InfiniteVirtualTable/antdColumns.tsweb/packages/agenta-ui/src/InfiniteVirtualTable/columnDef.tsweb/packages/agenta-ui/src/InfiniteVirtualTable/columns/buildEntityColumns.tsxweb/packages/agenta-ui/src/InfiniteVirtualTable/columns/createStandardColumns.tsxweb/packages/agenta-ui/src/InfiniteVirtualTable/columns/createTableColumns.tsweb/packages/agenta-ui/src/InfiniteVirtualTable/columns/types.tsweb/packages/agenta-ui/src/InfiniteVirtualTable/components/InfiniteVirtualTableInner.tsxweb/packages/agenta-ui/src/InfiniteVirtualTable/hooks/useColumnDomRefs.tsweb/packages/agenta-ui/src/InfiniteVirtualTable/hooks/useColumnVisibility.tsweb/packages/agenta-ui/src/InfiniteVirtualTable/hooks/useColumnVisibilityControls.tsweb/packages/agenta-ui/src/InfiniteVirtualTable/hooks/useResizableColumns.tsweb/packages/agenta-ui/src/InfiniteVirtualTable/hooks/useScopedColumnVisibility.tsxweb/packages/agenta-ui/src/InfiniteVirtualTable/hooks/useSmartResizableColumns.tsweb/packages/agenta-ui/src/InfiniteVirtualTable/hooks/useTableExport.tsweb/packages/agenta-ui/src/InfiniteVirtualTable/hooks/useTableHeaderHeight.tsweb/packages/agenta-ui/src/InfiniteVirtualTable/hooks/useTableManager.tsxweb/packages/agenta-ui/src/InfiniteVirtualTable/hooks/useTypeChipColumns.tsxweb/packages/agenta-ui/src/InfiniteVirtualTable/index.tsweb/packages/agenta-ui/src/InfiniteVirtualTable/types.tsweb/packages/agenta-ui/src/InfiniteVirtualTable/utils/columnUtils.tsweb/packages/agenta-ui/src/utils/groupColumns.ts
Comment on lines
+13
to
+15
| export type ColumnAlign = "start" | "end" | "left" | "right" | "center" | "justify" | "match-parent" | ||
|
|
||
| export type ColumnFixed = "start" | "end" | "left" | "right" | boolean |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n '"antd"\s*:' web/package.json
if [ -f web/pnpm-lock.yaml ]; then
rg -n 'rc-table@|rc-table:' web/pnpm-lock.yaml
fiRepository: Agenta-AI/agenta
Length of output: 154
🏁 Script executed:
set -euo pipefail
printf '%s\n' '--- candidate files ---'
git ls-files 'web/packages/agenta-ui/src/InfiniteVirtualTable/*' 'web/**/package.json' '*lock*' | head -200
printf '%s\n' '--- column definition ---'
cat -n web/packages/agenta-ui/src/InfiniteVirtualTable/columnDef.ts | sed -n '1,240p'
printf '%s\n' '--- dependency declarations ---'
find web -name package.json -print0 | xargs -0 grep -nE '"(antd|rc-table)"' || true
printf '%s\n' '--- lockfile references ---'
find . -maxdepth 3 -type f \( -name '*lock*' -o -name 'package.json' \) -print0 |
xargs -0 grep -nE '(^|[[:space:]])rc-table(@|:)|(^|[[:space:]])antd(@|:)' 2>/dev/null | head -200 || trueRepository: Agenta-AI/agenta
Length of output: 15079
🏁 Script executed:
set -euo pipefail
printf '%s\n' '--- adapter implementation ---'
cat -n web/packages/agenta-ui/src/InfiniteVirtualTable/antdColumns.ts | sed -n '1,280p'
printf '%s\n' '--- all align/fixed uses ---'
rg -n --glob 'web/packages/agenta-ui/src/InfiniteVirtualTable/**' '\b(align|fixed)\b|ColumnAlign|ColumnFixed' web/packages/agenta-ui/src/InfiniteVirtualTable
printf '%s\n' '--- package and lockfile dependency context ---'
sed -n '1,120p' web/packages/agenta-ui/package.json
sed -n '6750,6845p' web/pnpm-lock.yaml
rg -n -C 4 'rc-table(@|:)|antd@6\.3\.7|rc-util@|`@ant-design/icons`@' web/pnpm-lock.yaml | head -240Repository: Agenta-AI/agenta
Length of output: 20673
🏁 Script executed:
set -euo pipefail
printf '%s\n' '--- resolved antd dependency graph ---'
sed -n '16022,16085p' web/pnpm-lock.yaml
rg -n -C 3 '(`@rc-component/table`|rc-table)@' web/pnpm-lock.yaml || true
printf '%s\n' '--- table bridge call sites ---'
rg -n -C 5 'toAntdColumns|fromAntdColumns|<Table|AntdColumnsType' web/packages/agenta-ui/src/InfiniteVirtualTable web/packages/agenta-ui/src | head -260
printf '%s\n' '--- package manager metadata ---'
sed -n '1,90p' web/pnpm-lock.yaml
rg -n -C 3 '^ web/packages/agenta-ui:|antd:' web/pnpm-lock.yaml | head -100Repository: Agenta-AI/agenta
Length of output: 39551
🏁 Script executed:
set -euo pipefail
python3 - <<'PY'
from urllib.request import urlopen
from urllib.error import HTTPError
urls = [
"https://unpkg.com/antd@6.3.7/es/table/interface.d.ts",
"https://unpkg.com/@rc-component/table@1.9.1/es/interface.d.ts",
"https://unpkg.com/antd@5.0.0/es/table/interface.d.ts",
"https://unpkg.com/rc-table@7.32.2/es/interface.d.ts",
]
for url in urls:
print(f"--- {url} ---")
try:
text = urlopen(url, timeout=15).read().decode()
except Exception as exc:
print(f"FETCH_ERROR: {exc}")
continue
for i, line in enumerate(text.splitlines(), 1):
if any(token in line for token in ("align", "fixed", "AlignType", "FixedType")):
print(f"{i}:{line}")
PYRepository: Agenta-AI/agenta
Length of output: 998
🏁 Script executed:
set -euo pipefail
for url in \
'https://unpkg.com/antd@6.3.7/es/table/interface.d.ts' \
'https://unpkg.com/@rc-component/table@1.9.1/es/interface.d.ts' \
'https://unpkg.com/antd@5.0.0/es/table/interface.d.ts' \
'https://unpkg.com/rc-table@7.32.2/es/interface.d.ts'
do
printf '%s\n' "--- $url ---"
curl -k -fsSL --max-time 20 "$url" |
nl -ba |
grep -E 'align|fixed|AlignType|FixedType' || true
doneRepository: Agenta-AI/agenta
Length of output: 954
🏁 Script executed:
set -euo pipefail
python3 - <<'PY'
from urllib.request import urlopen
import ssl
ctx = ssl._create_unverified_context()
urls = [
"https://unpkg.com/antd@6.3.7/es/table/interface.d.ts",
"https://unpkg.com/@rc-component/table@1.9.1/es/interface.d.ts",
"https://unpkg.com/antd@5.0.0/es/table/interface.d.ts",
"https://unpkg.com/rc-table@7.32.2/es/interface.d.ts",
]
for url in urls:
print(f"--- {url} ---")
try:
text = urlopen(url, context=ctx, timeout=20).read().decode()
except Exception as exc:
print(f"FETCH_ERROR: {exc}")
continue
for i, line in enumerate(text.splitlines(), 1):
if any(token in line for token in ("align", "fixed", "AlignType", "FixedType")):
print(f"{i}:{line}")
PYRepository: Agenta-AI/agenta
Length of output: 1546
🏁 Script executed:
set -euo pipefail
python3 - <<'PY'
from urllib.request import urlopen
import json, ssl
ctx = ssl._create_unverified_context()
urls = [
"https://unpkg.com/antd@5.0.0/package.json",
"https://unpkg.com/antd@6.3.7/package.json",
"https://unpkg.com/rc-table@7.32.2/package.json",
"https://unpkg.com/@rc-component/table@1.9.1/package.json",
]
for url in urls:
print(f"--- {url} ---")
try:
data = json.loads(urlopen(url, context=ctx, timeout=20).read().decode())
except Exception as exc:
print(f"FETCH_ERROR: {exc}")
continue
print("version:", data.get("version"))
print("table dependency:", {k: v for k, v in data.get("dependencies", {}).items()
if "table" in k or k == "rc-util"})
PYRepository: Agenta-AI/agenta
Length of output: 613
🏁 Script executed:
set -euo pipefail
python3 - <<'PY'
from urllib.request import urlopen
import ssl
ctx = ssl._create_unverified_context()
for url in [
"https://unpkg.com/rc-table@7.26.0/es/interface.d.ts",
"https://unpkg.com/rc-table@7.26.0/package.json",
]:
print(f"--- {url} ---")
try:
text = urlopen(url, context=ctx, timeout=20).read().decode()
except Exception as exc:
print(f"FETCH_ERROR: {exc}")
continue
for i, line in enumerate(text.splitlines(), 1):
if any(token in line for token in ("FixedType", "AlignType", "fixed?:", "align?:")):
print(f"{i}:{line}")
PYRepository: Agenta-AI/agenta
Length of output: 475
🏁 Script executed:
set -euo pipefail
python3 - <<'PY'
from urllib.request import urlopen
import ssl, tarfile, io
ctx = ssl._create_unverified_context()
for package, version in [("rc-table", "7.26.0"), ("`@rc-component/table`", "1.9.1")]:
url = f"https://registry.npmjs.org/{package}/{version}"
print(f"--- {package}@{version} runtime references ---")
try:
meta = urlopen(url, context=ctx, timeout=20).read()
import json
data = json.loads(meta)
tarball = urlopen(data["dist"]["tarball"], context=ctx, timeout=20).read()
archive = tarfile.open(fileobj=io.BytesIO(tarball), mode="r:gz")
except Exception as exc:
print(f"FETCH_ERROR: {exc}")
continue
matches = 0
for member in archive.getmembers():
if not member.isfile() or not member.name.endswith((".js", ".mjs", ".ts", ".tsx")):
continue
text = archive.extractfile(member).read().decode("utf-8", "replace")
if "fixed ===" in text or "fixed ==" in text or "textAlign" in text or "align:" in text:
for i, line in enumerate(text.splitlines(), 1):
if any(token in line for token in ("fixed ===", "fixed ==", "textAlign", "align:")):
print(f"{member.name}:{i}:{line.strip()}")
matches += 1
if matches >= 30:
break
if matches >= 30:
break
PYRepository: Agenta-AI/agenta
Length of output: 6315
Preserve fixed-column behavior across the AntD peer range.
@agenta/ui supports antd >=5.0.0, but AntD 5.0.0 only fixes columns with true, "left", or "right". Restrict ColumnFixed to those values, or normalize "start" and "end" in toAntdColumns. AntD 6 supports the current union. ColumnAlign does not require this change.
|
|
||
| /** Internal column type with ordering metadata for sorting during grouping */ | ||
| type OrderedColumn<T> = ColumnType<T> & {__order?: number} | ||
| type OrderedColumn<T> = ColumnDef<T> & {__order?: number} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Return the grouped-column union.
groupColumnsRecursive adds {children} group columns, but its signatures report ColumnDef<T>[]. ColumnDef<T> is leaf-only. This erases the group-column contract from direct groupColumns consumers.
Return ColumnDefs<T> from both grouping functions. Type OrderedColumn<T> and countLeafColumns with the same union.
Proposed type fix
-import type {ColumnDef} from "../InfiniteVirtualTable/columnDef"
+import type {ColumnDef, ColumnDefs} from "../InfiniteVirtualTable/columnDef"
-type OrderedColumn<T> = ColumnDef<T> & {__order?: number}
+type OrderedColumn<T> = ColumnDefs<T>[number] & {__order?: number}
-function countLeafColumns<T>(columns: ColumnDef<T>[]): number {
+function countLeafColumns<T>(columns: ColumnDefs<T>): number {
-): ColumnDef<T>[] {
- const result: ColumnDef<T>[] = []
+): ColumnDefs<T> {
+ const result: ColumnDefs<T> = []
-): ColumnDef<T>[] {
+): ColumnDefs<T> {Also applies to: 102-105, 121-127, 136-136, 252-256
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
InfiniteVirtualTableis typed against antd'sColumnsType. That type reaches 22 files and 160 references across packages and both apps, and every new table column adds to it. The table itself has to come off antd eventually, since the target architecture is one antd-free app, and this type surface is what blocks every step of that port.This is step 1 of the table port from
docs/design/observability-packages/plan.md§8, pulled forward because it is mechanical, carries no visual risk, and lands independently.Changes
ColumnDef<T>now lives inpackages/agenta-ui/src/InfiniteVirtualTable/columnDef.ts, alongsideColumnGroupDef,ColumnDefsand the supporting scalars. antd'sColumnsTypebecomes an adapter applied at exactly one place:The table's rendering does not move.
<Table virtual>stays exactly as it is. Only the type surface changes, so the later steps stop being blocked on a 22-file refactor.One decision worth a reviewer's eye. Every function-valued column prop (
render,onCell,onHeaderCell,shouldCellUpdate,onFilter) is declared with method syntax, not property syntax:Method parameters are checked bivariantly, so a column may still write
render: (date: string) => …and stay assignable. That is what antd was buying withany, obtained here without one. Property syntax withunknownwould have broken about fifty call sites.getObservabilityColumns.tsxalso drops its last antd import, swappingTagfor theSpanIdChipbuilt inobs/wp3.Tests / notes
@agenta/uitypecheck.@agenta/ui,@agenta/entity-uiand@agenta/observability-uibuild and lint.@agenta/entity-ui's 325 tests pass.packages/agenta-ui/src/utils/groupColumns.tssits outside the directory that was measured but is antd-typed and feedsbuildEntityColumns, so it had to move here or the package would not compile. And the observabilitytypes.d.tsantd import did not fall out for free: it also importedAvatarfor an exportedAvatarTreeContentProps, which turned out to have no references anywhere inoss,ee,packagesormobile, so it is deleted.fromAntdColumnsis exported but currently unused. It exists for the four raw<Table>call sites deliberately left on antd (MetadataSummaryTable,getAnnotationTableColumns,ConfigurationTable,DeploymentHistoryModal).What to QA
No visual change is expected anywhere. The risk is a column silently losing its renderer, so spot-check the tables that were retyped.