fix: remove references to sampleId field removed from Azul backend - #4928
Conversation
There was a problem hiding this comment.
Pull request overview
Removes obsolete Azul sampleId usage to restore HCA DCP and LungMAP sample listings.
Changes:
- Removes Sample ID columns, labels, builders, and response fields.
- Changes sample-table default sorting to
projectTitle. - Cleans up obsolete file-table visibility settings.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
site-config/lungmap/dev/index/samplesEntityConfig.ts |
Removes Sample ID column and updates sorting. |
site-config/lungmap/dev/index/filesEntityConfig.ts |
Removes Sample ID column and visibility entry. |
site-config/hca-dcp/ma-dev/entities/samples/tableOptions.ts |
Updates default sample sorting. |
site-config/hca-dcp/ma-dev/entities/samples/columns.ts |
Removes Sample ID column. |
site-config/hca-dcp/ma-dev/entities/files/tableOptions.ts |
Removes obsolete visibility entry. |
site-config/hca-dcp/ma-dev/entities/files/columns.ts |
Removes Sample ID column. |
site-config/hca-dcp/category.ts |
Removes Sample ID category key and label. |
app/viewModelBuilders/azul/hca-dcp/common/viewModelBuilders.tsx |
Removes obsolete Sample ID builders. |
app/components/Index/common/entities.ts |
Removes Sample ID metadata key. |
app/components/Index/common/constants.ts |
Removes Sample ID pluralized label. |
app/apis/azul/hca-dcp/common/entities.ts |
Aligns sample response type with Azul. |
app/apis/azul/hca-dcp/common/aggregatedEntities.ts |
Aligns aggregated sample type with Azul. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
frano-m
left a comment
There was a problem hiding this comment.
Hi Hannes! 👋 (Claude here, writing on behalf of Fran)
Really nice work — the removal is impressively thorough: no dangling SAMPLE_ID / buildSampleId references, prod configs inherit the fix, and the live-verified default sort was appreciated.
One small thing before merge: the fixture in __tests__/utils/schemaOrg/hcaProjectDataset.test.ts:253 still has id: ["s1"] in the samples array — the field this PR removes. It slips past tsc via the as unknown as Partial<ProjectsResponse> cast.
With that, approved from our side. Thanks again! 🙌
— Fran (via Claude)
The fixture in the buildHcaProjectJsonLd test still set `id` on the aggregated samples entry, a field removed from `AggregatedSampleResponse` in this branch. The `as unknown as Partial<ProjectsResponse>` double assertion suppressed the resulting type error, so `tsc` did not flag it. `buildKeywords` never reads `id`, so no assertion changes. The `id` on the adjacent specimens entry is left alone: `AggregatedSpecimenResponse` still declares it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`AggregatedSpecimenResponse.id` no longer exists in the Azul response. Verified against catalog dcp60: across 74 inner `specimens` objects from /index/projects, /index/files and /index/samples, the keys are only disease, organ, organPart, preservationMethod and source. No code read the field — the three readers of `specimens` use disease, organ and organPart — so this is a type-accuracy fix with no behavioural change. The matching `id` in the buildHcaProjectJsonLd fixture goes too, since leaving it would reintroduce the same silently-cast stale property. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The fixture in the buildHcaProjectJsonLd test still set `id` on the aggregated samples entry, a field removed from `AggregatedSampleResponse` in this branch. The `as unknown as Partial<ProjectsResponse>` double assertion suppressed the resulting type error, so `tsc` did not flag it. `buildKeywords` never reads `id`, so no assertion changes. The `id` on the adjacent specimens entry is left alone: `AggregatedSpecimenResponse` still declares it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`AggregatedSpecimenResponse.id` no longer exists in the Azul response. Verified against catalog dcp60: across 74 inner `specimens` objects from /index/projects, /index/files and /index/samples, the keys are only disease, organ, organPart, preservationMethod and source. No code read the field — the three readers of `specimens` use disease, organ and organPart — so this is a type-accuracy fix with no behavioural change. The matching `id` in the buildHcaProjectJsonLd fixture goes too, since leaving it would reintroduce the same silently-cast stale property. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
6384d17 to
dcad74b
Compare
The Azul backend no longer supports the `sampleId` field, and the inner `samples[].id` property is likewise absent from `/index/samples` and `/index/files` responses. This broke the HCA and LungMAP samples tables, which sorted by `sampleId` by default and so failed with `400 BadRequestError: Unknown field 'sampleId'`, and left the Sample Id columns in the samples and files tables rendering empty values. Remove the Sample Id columns from the HCA and LungMAP samples and files tables, along with the category key and label, the `buildSampleId` and `buildAggregatedSampleId` view model builders, the `SAMPLE_ID` metadata key, and the `id` property on `SampleResponse` and `AggregatedSampleResponse`. Default sorting on both samples tables changes to `projectTitle`, now the leading column, matching the files tables which sort by their own leading column. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The fixture in the buildHcaProjectJsonLd test still set `id` on the aggregated samples entry, a field removed from `AggregatedSampleResponse` in this branch. The `as unknown as Partial<ProjectsResponse>` double assertion suppressed the resulting type error, so `tsc` did not flag it. `buildKeywords` never reads `id`, so no assertion changes. The `id` on the adjacent specimens entry is left alone: `AggregatedSpecimenResponse` still declares it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`AggregatedSpecimenResponse.id` no longer exists in the Azul response. Verified against catalog dcp60: across 74 inner `specimens` objects from /index/projects, /index/files and /index/samples, the keys are only disease, organ, organPart, preservationMethod and source. No code read the field — the three readers of `specimens` use disease, organ and organPart — so this is a type-accuracy fix with no behavioural change. The matching `id` in the buildHcaProjectJsonLd fixture goes too, since leaving it would reintroduce the same silently-cast stale property. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dcad74b to
9f2f3df
Compare
Filed by Claude Code on behalf of @hannes-ucsc.
Problem
The Azul backend no longer supports the
sampleIdfield. This broke the samples tables, which sorted by it by default:Reproduced live against
service.azul.data.humancellatlas.org(catalogdcp60). Affected https://explore.data.humancellatlas.org/samples and the equivalent LungMAP samples page.The inner
samples[].idproperty is also gone from responses./index/samplesand/index/filesnow return onlyaccessible,sampleEntityType,effectiveOrgan,organ,organPart,disease,preservationMethod,source,modelOrgan,modelOrganPart. So the field was unusable both as a sort key and as a displayed value — the Sample Id columns would have rendered empty even where they didn't 400. They are removed rather than re-keyed.Changes
SAMPLE_ID: "sampleId"and its"Sample Id"label fromsite-config/hca-dcp/category.ts.columnVisibilityentries in both files tables.sampleId, which is what produced the 400. Changed toprojectTitle, now the leading column — consistent with the files tables, which sort by their own leading column (fileName). Confirmedsort=projectTitlereturns 200 against the samples index.buildSampleIdandbuildAggregatedSampleId; they were the only readers ofsamples[].id.idfromSampleResponseandAggregatedSampleResponse, and removedMETADATA_KEY.SAMPLE_IDplus its"sample ids"pluralized label (used only by the deleted aggregated builder).AnVIL's
biosamples.biosample_id("BioSample Id") is a different backend field and is untouched.Verification
tsc --noEmit— cleannpm run lint— 0 errorsprettier --check— cleanjest --ci— 129/129 passinggit grep sampleId— zero hits across.ts/.tsx(excluding AnVIL'sbiosample_id)Note for reviewers
This fix has already been merged into the deployment branches (
ucsc/hca/dev,ucsc/hca/prod,ucsc/lungmap/dev,ucsc/lungmap/prod) and pushed to their GitLab remotes, ahead of this PR, to unbreak the live samples pages. This PR brings the change tomainso a future release merge doesn't revert it. The threeucsc/anvil/*branches deliberately do not carry it.projectTitleas the replacement default sort is a judgement call, not something the field removal dictated. This choice is temporary. Azul will soon introduce support for the the more meaningfulbiomaterial_namefield (primarily requested by LungMAP, but useful on HCA as well) which we'll recommend to become the default sort for the samples tab.