Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR reworks “column metadata” across the stack to support richer per-column configuration (semantic types, value color mappings, inspector visibility/formatting), introduce organisation-level overrides, and update the map inspector + dashboards to use the new structures.
Changes:
- Add
inspectorColumns,semanticType, andvalueColorssupport toDataSource/metadata, plus UI editors for labels and color mappings. - Replace
columnMetadataOverridewith a newDataSourceOrganisationOverridemodel and tRPC endpoints for patching metadata at both owner/org-override levels. - Introduce percentage semantic-type inference during import and migrate stored
categoryColors/colorMappingsand metadata color fields.
Reviewed changes
Copilot reviewed 75 out of 91 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/utils/superjson.test.ts | Update serializer fixture with inspectorColumns. |
| tests/unit/utils/resolveColumnMetadata.test.ts | Add tests for valueColors merge behaviour. |
| tests/unit/server/trpc/routers/dataSource.test.ts | Update router fixtures with inspectorColumns. |
| tests/unit/server/trpc/routers/dataRecord.test.ts | Update router fixtures with inspectorColumns. |
| tests/unit/server/trpc/routers/auth.test.ts | Update rate limit import path for mocks. |
| tests/unit/server/repositories/DataRecord.test.ts | Update fixtures with inspectorColumns. |
| tests/unit/server/jobs/importDataSource.test.ts | New unit tests for percentage inference helpers. |
| tests/unit/app/api/login/route.test.ts | Update rate limit import path for mocks. |
| tests/resources/percentages.csv | New CSV fixture for semantic inference tests. |
| tests/feature/stats.test.ts | Update fixtures with inspectorColumns. |
| tests/feature/importDataSource.test.ts | Add feature tests for semantic-type inference + preservation. |
| tests/feature/geojsonAPI.test.ts | Update fixtures with inspectorColumns. |
| tests/feature/enrichDataSource.test.ts | Update fixtures with inspectorColumns. |
| tests/feature/dataSourcesAPI.test.ts | Update fixtures with inspectorColumns. |
| src/utils/resolveColumnMetadata.ts | Merge valueColors during metadata resolution. |
| src/utils/colors.ts | New shared category → default color scale helper. |
| src/server/trpc/routers/dataSource.ts | Switch to org overrides model; add patch endpoints; persist inspectorColumns. |
| src/server/trpc/routers/auth.ts | Move rate limit import to server/services/ratelimit. |
| src/server/trpc/index.ts | Move getClientIp import to server/services/ratelimit. |
| src/server/services/ratelimit.ts | New shared rate-limit + IP helper module. |
| src/server/services/database/schema.ts | Add inspectorColumns to DB schema typings. |
| src/server/services/database/index.ts | Swap DB table typing from old override table to new. |
| src/server/repositories/DataSourceOrganisationOverride.ts | Rename repo functions; add inspectorColumns persistence. |
| src/server/repositories/DataRecord.ts | Add numeric range query for semantic inference. |
| src/server/models/DataSourceOrganisationOverride.ts | New Kysely table types for org override table. |
| src/server/models/ColumnMetadataOverride.ts | Remove old Kysely table types. |
| src/server/jobs/importDataSource.ts | Add semantic-type inference (percentage) after import. |
| src/server/jobs/importDataRecords.ts | Run semantic-type inference after record import job. |
| src/server/commands/ensureOrganisationMap.ts | Ensure created data sources include inspectorColumns. |
| src/models/MapView.ts | Rename inspector config schema (boundaries → dataSources) and categoryColors → colorMappings. |
| src/models/DataSourceOrganisationOverride.ts | New Zod schema/type for org overrides. |
| src/models/DataSource.ts | Add semantic/display enums; add valueColors + inspectorColumns schemas/types. |
| src/models/ColumnMetadataOverride.ts | Remove old Zod schema/type. |
| src/labels.ts | Add labels for semantic type + display format enums. |
| src/hooks/useColumnValues.ts | New hook to fetch/sort unique values with nullIsZero normalization. |
| src/constants/index.ts | Re-export color constants. |
| src/constants/colors.ts | Centralize party/category color constants. |
| src/components/ValueLabelsEditor.tsx | New reusable editor for value → label mappings. |
| src/components/ColorMappingsEditor.tsx | New reusable editor for value → color mappings. |
| src/app/api/login/route.ts | Move rate limit import to server/services/ratelimit. |
| src/app/(private)/map/[id]/hooks/useInspector.ts | Update inspector component import path. |
| src/app/(private)/map/[id]/hooks/useInitialMapView.ts | Init inspector config with dataSources key. |
| src/app/(private)/map/[id]/hooks/useDisplayAreaStats.ts | Resolve metadata via org override; pass valueColors to fill color. |
| src/app/(private)/map/[id]/hooks/useColumnMetadataMutations.ts | New hook for metadata patch mutations + cache updates. |
| src/app/(private)/map/[id]/constants.ts | Re-export centralized party colors; keep default fill constant. |
| src/app/(private)/map/[id]/components/controls/VisualisationPanel/VisualisationPanel.tsx | Switch to valueColors and org override metadata source. |
| src/app/(private)/map/[id]/components/PrivateMapOverlay.tsx | Update EditColumnMetadataModal import path. |
| src/app/(private)/map/[id]/components/PrivateMapControls.tsx | Update inspector + hover info import paths. |
| src/app/(private)/map/[id]/components/Markers/Markers.tsx | New marker layer component wrapper. |
| src/app/(private)/map/[id]/components/MapViews.tsx | Init inspector config with dataSources key. |
| src/app/(private)/map/[id]/components/Map.tsx | Update Choropleth/Markers import paths to new directories. |
| src/app/(private)/map/[id]/components/Legend.tsx | Use valueColors/org overrides for categoric schemes + editor fields. |
| src/app/(private)/map/[id]/components/InspectorPanel/helpers.ts | New helper utilities for inspector geometry + grouping. |
| src/app/(private)/map/[id]/components/InspectorPanel/UnderlineTabs.tsx | New animated underline tabs component. |
| src/app/(private)/map/[id]/components/InspectorPanel/TurfMarkersList.tsx | New inspector tab content for turf marker lists. |
| src/app/(private)/map/[id]/components/InspectorPanel/SimplePropertiesList.tsx | New property list renderer with tooltip descriptions. |
| src/app/(private)/map/[id]/components/InspectorPanel/MarkersLists.tsx | New shared marker list components. |
| src/app/(private)/map/[id]/components/InspectorPanel/MarkerButton.tsx | New marker list button component. |
| src/app/(private)/map/[id]/components/InspectorPanel/InspectorPanel.tsx | New inspector panel implementation with tabs + boundary actions. |
| src/app/(private)/map/[id]/components/InspectorPanel/InspectorNotesTab.tsx | New (placeholder) notes tab. |
| src/app/(private)/map/[id]/components/InspectorPanel/InspectorMarkersTab.tsx | New markers tab switcher by selected layer type. |
| src/app/(private)/map/[id]/components/InspectorPanel/InspectorDataTab.tsx | Update inspector config key from boundaries → dataSources. |
| src/app/(private)/map/[id]/components/InspectorPanel/InspectorDataConfig.tsx | Update inspector config editing to dataSources schema/types. |
| src/app/(private)/map/[id]/components/InspectorPanel/DataSourcePropertiesList.tsx | Resolve metadata via org override object. |
| src/app/(private)/map/[id]/components/InspectorPanel/ClusterMarkersList.tsx | New cluster marker list tab content. |
| src/app/(private)/map/[id]/components/InspectorPanel/BoundaryMarkersList.tsx | New boundary marker list tab content (client-side filtering). |
| src/app/(private)/map/[id]/components/InspectorPanel/BoundaryDataPanel.tsx | Update inspector config type name. |
| src/app/(private)/map/[id]/components/InspectorPanel/BoundaryConfigItem.tsx | Update inspector config type name. |
| src/app/(private)/map/[id]/components/EditColumnMetadataModal/ValueLabelsSection.tsx | New section wrapper using shared ValueLabelsEditor. |
| src/app/(private)/map/[id]/components/EditColumnMetadataModal/EditColumnMetadataModal.tsx | New modal split into sections; uses patch endpoints + shared hooks. |
| src/app/(private)/map/[id]/components/EditColumnMetadataModal/ColorMappingsSection.tsx | New map-view color override editor with save-as-defaults behavior. |
| src/app/(private)/map/[id]/components/EditColumnMetadataModal.tsx | Remove old monolithic modal implementation. |
| src/app/(private)/map/[id]/components/ColumnMetadataIcons.tsx | Resolve metadata via org override object. |
| src/app/(private)/map/[id]/components/Choropleth/useChoroplethColors.ts | Pass resolved valueColors into choropleth fill computation. |
| src/app/(private)/map/[id]/components/Choropleth/Choropleth.tsx | New component file in directory structure. |
| src/app/(private)/map/[id]/components/BoundaryHoverInfo/BoundaryHoverInfo.tsx | New component file in directory structure. |
| src/app/(private)/map/[id]/colors.ts | Use colorMappings + resolved value colors; centralize default category colors. |
| src/app/(private)/map/[id]/atoms/editColumnMetadataAtom.ts | Rename field selector from categoryColors → valueColors. |
| src/app/(private)/(dashboards)/data-sources/[id]/components/EnrichmentTable.tsx | Rename/export default; adjust dialog import; tweak invalidation. |
| src/app/(private)/(dashboards)/data-sources/[id]/components/EnrichmentColumnDialog.tsx | Move/rename enrichment dialog implementation. |
| src/app/(private)/(dashboards)/data-sources/[id]/components/ColumnVisualisationPanel.tsx | New UI for configuring inspectorColumns + inspector preview. |
| src/app/(private)/(dashboards)/data-sources/[id]/components/ColumnMetadataTable.tsx | New UI for descriptions, semantic types, labels, and colors. |
| src/app/(private)/(dashboards)/data-sources/[id]/components/ColumnMetadataForm.tsx | Remove old metadata dialog-based form. |
| src/app/(private)/(dashboards)/data-sources/[id]/DataSourceDashboard.tsx | Add new tabs for metadata/inspector; invalidate byId on ImportComplete. |
| migrations/1774300000000_rename_column_metadata_color_mappings.ts | Migrate colorMappings → valueColors in column metadata JSON. |
| migrations/1774222928000_rename_map_view_category_colors.ts | Migrate map view config categoryColors → colorMappings. |
| migrations/1774206346033_rename_column_visualisations_to_inspector_columns.ts | Rename DB columns to inspectorColumns. |
| migrations/1774206346032_inspector_config_boundaries_to_data_sources.ts | Migrate inspector config key boundaries → dataSources. |
| migrations/1774204667147_data_source_organisation_override.ts | Rename override table and add visualisations column. |
| migrations/1774204667146_data_source_column_visualisations.ts | Add initial columnVisualisations column to dataSource. |
| AGENTS.md | Add documented style/dialect conventions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+446
to
+451
| sql<number | null>`MIN((json->>${columnName})::float)`.as("min"), | ||
| sql<number | null>`MAX((json->>${columnName})::float)`.as("max"), | ||
| sql<boolean>`BOOL_OR((json->>${columnName})::float > 0 AND (json->>${columnName})::float < 1)`.as( | ||
| "hasDecimals", | ||
| ), | ||
| ]) |
There was a problem hiding this comment.
getNumericColumnRange casts (json->>columnName) directly to float. If imported JSON contains empty strings (common for blank CSV cells) or any non-numeric text, Postgres will throw on the cast and this will crash semantic-type inference (and potentially the import job). Consider guarding the cast with NULLIF(..., '') (and optionally filtering with a numeric regex / try_cast-style pattern) so invalid/blank values are treated as NULL rather than failing the query.
Comment on lines
+319
to
+322
| for (const col of numericColumns) { | ||
| console.log("checking col", col); | ||
| const existing = updatedMetadata.find((m) => m.name === col.name); | ||
| if (existing?.semanticType !== undefined) continue; |
There was a problem hiding this comment.
Leftover console.log in inferColumnSemanticTypes will spam server logs during imports. Please remove it (or replace with the repo logger behind an appropriate debug flag).
Comment on lines
+311
to
+328
| const numericColumns = dataSource.columnDefs.filter( | ||
| (col) => col.type === ColumnType.Number, | ||
| ); | ||
| if (numericColumns.length === 0) return; | ||
|
|
||
| const updatedMetadata: ColumnMetadata[] = [...dataSource.columnMetadata]; | ||
| let changed = false; | ||
|
|
||
| for (const col of numericColumns) { | ||
| console.log("checking col", col); | ||
| const existing = updatedMetadata.find((m) => m.name === col.name); | ||
| if (existing?.semanticType !== undefined) continue; | ||
|
|
||
| const hasPercentageColumnName = isPercentageColumnName(col.name); | ||
| const { min, max, hasDecimals } = await getNumericColumnRange( | ||
| dataSource.id, | ||
| col.name, | ||
| ); |
There was a problem hiding this comment.
inferColumnSemanticTypes runs getNumericColumnRange in a loop, which issues one full-table aggregate query per numeric column. On wide datasets this can become very slow (multiple sequential scans of dataRecord.json) and materially increase import time. Consider reducing queries (e.g., only infer for percentage-like names, batching multiple columns per query, or computing ranges during the existing import scan).
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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
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.
the data model summary is this:
DataSource
3 properties that contain column information:
columnDefs- core column type definitions, derived from import. cannot be modified by the user as this will break visualisations (e.g. if a user says a column is a number when it contains non-numeric values, this will break the choropleth)columnMetadata- extra column info, automatically set with best guess on first import, then controlled by the userdescriptionsemanticType- right now just hasAuto,Percentage01andPercentage0100so we know if the data is a % from 0-1 or a % from 0-100 without having to guess from the datavalueLabels- a way for the user to override what a value is displayed as, e.g. "0" => "Not in cluster"valueColors- default category colors for the columninspectorColumns- default inspector config for the data sourcedisplayFormat- this may seem similar tosemanticType, but it's different, because it is specific to the inspectorThe key thing to keep in mind is that
columnMetadatashould be used when the information is relevant to a column whenever it is used or displayed, andinspectorColumnsshould be used when the information is only relevant to the inspector.DataSourceOrganisationOverride
MapView
colorMapping- overrides thevalueColorsfor data source columns