Fix: Stitched HCA bundles are indexed as many links replicas that differ only in link order (#8393, #8259, #8294) by achave11-ucsc · Pull Request #8418 · DataBiosphere/azul

achave11-ucsc · 2026-10-20T13:38:17Z

Generated with Claude Code

Linked issues: #8393, #8259, #8294

Author

¹ when the issue title describes a problem, the corresponding PR
title is Fix: followed by the issue title

Author (partiality)

B01 Added p tag to titles of partial commits
B02 This PR is labeled partial _{or completely resolves all linked issues}
B03 This PR partially resolves each of the linked issues _{or does not have the partial label}

Author (reindex)

C01 Added r tag to commit title _{or the changes introduced by this PR will not require reindexing of any deployment}
C02 This PR is labeled reindex:dev _{or the changes introduced by it will not require reindexing of dev}
C03 This PR is labeled reindex:anvildev _{or the changes introduced by it will not require reindexing of anvildev}
C04 This PR is labeled reindex:anvilprod _{or the changes introduced by it will not require reindexing of anvilprod}
C05 This PR is labeled reindex:prod _{or the changes introduced by it will not require reindexing of prod}
C06 This PR is labeled reindex:partial and its description documents the specific reindexing procedure for dev, anvildev, anvilprod and prod _{or requires a full reindex or carries none of the labels reindex:dev, reindex:anvildev, reindex:anvilprod and reindex:prod}

Author (mirror)

D01 This PR is labeled mirror:dev _{or the changes introduced by it will not require mirroring of dev}
D02 This PR is labeled mirror:anvildev _{or the changes introduced by it will not require mirroring of anvildev}
D03 This PR is labeled mirror:anvilprod _{or the changes introduced by it will not require mirroring of anvilprod}
D04 This PR is labeled mirror:prod _{or the changes introduced by it will not require mirroring of prod}
D05 This PR is labeled mirror:partial and its description documents the specific mirroring procedure for dev, anvildev, anvilprod and prod _{or requires a full mirroring or carries none of the labels mirror:dev, mirror:anvildev, mirror:anvilprod and mirror:prod}

Author (API changes)

E01 This PR and its linked issues are labeled API _{or this PR does not modify a REST API}
E02 Added a (A) tag to commit title for backwards (in)compatible changes _{or this PR does not modify a REST API}
E03 Updated REST API version number in app.py _{or this PR does not modify a REST API}

Author (upgrading deployments)

F01 Ran make docker_images.json and committed the resulting changes _{or this PR does not modify azul_docker_images, or any other variables referenced in the definition of that variable}
F02 Documented upgrading of deployments in UPGRADING.rst _{or this PR does not require upgrading deployments}
F03 Added u tag to commit title _{or this PR does not require upgrading deployments}
F04 This PR is labeled upgrade _{or does not require upgrading deployments}
F05 This PR is labeled deploy:shared _{or does not modify docker_images.json, and does not require deploying the shared component for any other reason}
F06 This PR is labeled deploy:gitlab _{or does not require deploying the gitlab component}
F07 This PR is labeled deploy:runner _{or does not require deploying the runner image}

Author (hotfixes)

G01 Added F tag to main commit title _{or this PR does not include permanent fix for a temporary hotfix}
G02 Reverted the temporary hotfixes for any linked issues _{or the none of the stable branches (anvilprod and prod) have temporary hotfixes for any of the issues linked to this PR}

Author (before every review)

H01 Rebased PR branch on develop, squashed fixups from prior reviews
H02 Ran make requirements_update _{or this PR does not modify pyproject.toml}
H03 Added R tag to commit title _{or this PR does not modify uv.lock}
H04 This PR is labeled reqs _{or does not modify uv.lock}
H05 make integration_test passes in personal deployment _{or this PR does not modify functionality that could affect the IT outcome}
H06 PR is awaiting requested review from a peer
H07 Status of PR is Review requested
H08 PR is assigned to only the peer and the author

Peer reviewer (after approval)

Note that after requesting changes, the PR must be assigned to only the author.

J01 Actually approved the PR
J02 PR is not a draft
J03 PR is awaiting requested review from system administrator
J04 Status of PR is Review requested
J05 PR is assigned to only the system administrator and the author

System administrator (after approval)

K01 Actually approved the PR
K02 Labeled linked issues as demo or no demo
K03 Commented on linked issues about demo expectations _{or all linked issues are labeled no demo}
K04 Decided if PR can be labeled no sandbox
K05 A comment to this PR details the completed security design review
K06 PR title is appropriate as title of merge commit
K07 N reviews label is accurate
K08 Status of PR is Approved
K09 PR is assigned to only the operator and the author

Operator

L01 Checked reindex:… labels and r commit title tag
L02 Checked mirror:… labels
L03 Checked that demo expectations are clear _{or all linked issues are labeled no demo}
L04 Squashed PR branch and rebased onto develop
L05 Sanity-checked history
L06 Pushed PR branch to GitHub

Operator (deploy `.shared` and `.gitlab` components)

M01 Ran _select dev.shared && CI_COMMIT_REF_NAME=develop make -C terraform/shared apply_keep_unused _{or this PR is not labeled deploy:shared}
M02 Ran _select dev.gitlab && CI_COMMIT_REF_NAME=develop make -C terraform/gitlab apply(an error from _login_docker_gitlab is benign if the instance was stopped for backup) _{or this PR is not labeled deploy:gitlab}
M03 Ran _select anvildev.shared && CI_COMMIT_REF_NAME=develop make -C terraform/shared apply_keep_unused _{or this PR is not labeled deploy:shared}
M04 Ran _select anvildev.gitlab && CI_COMMIT_REF_NAME=develop make -C terraform/gitlab apply(an error from _login_docker_gitlab is benign if the instance was stopped for backup) _{or this PR is not labeled deploy:gitlab}
M05 Checked the items in the next section _{or this PR is labeled deploy:gitlab}
M06 PR is assigned to only the system administrator and the author _{or this PR is not labeled deploy:gitlab}

System administrator (post-deploy of `.gitlab` component)

N01 Background migrations for dev.gitlab are complete _{or this PR is not labeled deploy:gitlab}
N02 Background migrations for anvildev.gitlab are complete _{or this PR is not labeled deploy:gitlab}
N03 PR is assigned to only the operator and the author

Operator (deploy runner image)

P01 Ran _select dev.gitlab && make -C terraform/gitlab/runner _{or this PR is not labeled deploy:runner}
P02 Ran _select anvildev.gitlab && make -C terraform/gitlab/runner _{or this PR is not labeled deploy:runner}

Operator (sandbox build)

Operator (merge the branch)

R01 All status checks passed and the PR is mergeable
R02 The title of the merge commit starts with the title of this PR
R03 Added PR # reference to merge commit title
R04 Collected commit title tags in merge commit title _{but only included p if the PR is also labeled partial}
R05 Pushed merge commit to GitHub
R06 Status of PR is Merged lower
R07 Status of blocked issues is Triage _{or no issues are blocked on the linked issues}

Operator (main build)

Operator (reindex)

Operator (mirroring)

U01 Started mirroring in dev _{or this PR is not labelled mirror:dev}
U02 Started mirroring in anvildev _{or this PR is not labelled mirror:anvildev}
U03 Checked for, triaged and possibly requeued messages in mirror fail queue in dev _{or this PR is not labelled mirror:dev}
U04 Checked for, triaged and possibly requeued messages in mirror fail queue in anvildev _{or this PR is not labelled mirror:anvildev}
U05 Emptied mirror fail queue in dev _{or this PR is not labelled mirror:dev}
U06 Emptied mirror fail queue in anvildev _{or this PR is not labelled mirror:anvildev}

Operator

V01 Propagated the upgrade, API, deploy:shared, deploy:gitlab, deploy:runner, reindex:partial, reindex:anvilprod, reindex:prod, mirror:partial, mirror:anvilprod and mirror:prod labels to any open promotion PRs _{or this PR carries none of these labels, or is not included in an open promotion PR}
V02 Propagated any specific instructions related to those labels, from the description of this PR to that of any open promotion PRs _{or this PR carries none of those labels, or is not included in an open promotion PR}
V03 PR is assigned to no one

Shorthand for review comments

L line is too long
W line wrapping is wrong
Q bad quotes
F other formatting problem

codecov · 2026-10-20T13:51:24Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.00%. Comparing base (5e6a3f1) to head (dfe1faf).

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #8418   +/-   ##
========================================
  Coverage    84.99%   85.00%           
========================================
  Files          170      170           
  Lines        26068    26085   +17     
========================================
+ Hits         22157    22174   +17     
  Misses        3911     3911

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

coveralls · 2026-10-20T13:52:21Z

coverage: 85.077% (+0.009%) from 85.068% — issues/achave11-ucsc/8393-8259-8294-stitched-links-order into develop

achave11-ucsc · 2026-10-20T20:38:44Z

-            source_contents = [row['content'] for row in links_jsons]
+            # The stitched bundles are in the iteration order of a set, which
+            # varies between processes.
+            stitched.sort(key=itemgetter('links_id'))


This comment was written by Claude Code.

stitched is a new list, created by the unpacking above, so sorting it in place leaves the caller's list untouched and has the same effect as sorted(). The file uses .sort() for a list it owns (rows.sort(key=itemgetter(pk_column)) in _emulate_bundle) and sorted() for an iterable (sorted(iter_rows, key=key) in _query_unique_sorted).

The `stitched` attribute of a canned HCA bundle is a set of the UUIDs of the entities that were stitched into that bundle. The canned TDR HCA bundle 4426adc5 also listed, for each stitched file, the UUID of that file's manifest entry. The plugin only ever adds an entity's own UUID, so the canner cannot have produced those six surplus entries. They were inert, because `Bundle.not_stitched` matches the set against each entity's `document_id`, which a manifest entry UUID never equals. The surplus went unnoticed because `_test_fetch_bundle` compared every other attribute of a canned bundle against that of the emulated one, but not `stitched`. It now does. The canned DSS bundles carry the same surplus, tracked in #8423. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The comment stated that the manifest and the metadata of a bundle are both sorted by entity UUID. The assertions below it compare dictionaries, which are equal regardless of the order of their entries, so nothing verified that claim. The claim was inaccurate, too. The entries are grouped by entity type and only sorted by UUID within such a group. The order of the groups is that of a set's iteration. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

A plugin groups those entries by entity type, in the iteration order of a set, which varies with Python's hash seed. Canning one bundle twice could therefore yield two files that differ only in the order of their entries. The canner now sorts both by entity reference. The entries of the canned TDR HCA bundles are reordered accordingly. They are otherwise unchanged, and so are the other attributes of those bundles. Only the canned file is normalized. The order of the entries of a bundle in memory still varies between processes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The canned bundles of `test_subgraph_stitching` formed a chain, so the plugin retrieved them one at a time, in the order the chain dictated. The order of the merged links only varies when one batch holds more than one bundle, a shape that fixture could not produce. The root bundle now consumes a second file, produced by a new bundle whose own input is a cell suspension rather than a file. That bundle is therefore self-contained, and the traversal retrieves it together with the first upstream bundle, in one batch. A batch is a set, whose iteration order varies between processes, so this test now fails in roughly half of all runs, until the commit that merges the links in a deterministic order. The new bundle, its process and the file it produces are synthetic. All other entities they reference already exist in the canned tables. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The stitched bundles of one batch are the elements of a set, so the order in which the plugin emits them, and with it the order of the merged links, varies between processes. The preceding commit made the fixture produce such a batch, but a single run of the test still covers only whichever order that process happens to produce. The new test merges the links of the stitched bundles in every permutation and expects the result of the canned bundle each time. It fails until the next commit, which merges them in a deterministic order. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The TDR HCA plugin merged the links in the order in which it retrieved the stitched bundles. That is the iteration order of a set and differs between indexer invocations, so every partition of a stitched bundle could produce a links replica with a different content hash. In prod, one bundle has 101 replicas that differ only in the order of their links. The links of the root bundle now come first, followed by those of the other bundles in the order of their UUID. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

A test used to drop the tables it had created, so that the next test of the class could create them again in the same emulator. Dropping them grew more expensive the longer that emulator had been in use, from 4.7s in the first test of a class to 11.6s in the fifth, and dominated the run time of both affected test classes. A test now gets an emulator of its own and discards it afterwards. Launching one costs 0.3s and makes dropping the tables unnecessary. Running test_tdr and test_anvil together takes 59s instead of 142s, for the same 29 tests. Replacing the nineteen table deletions with a single deletion of the dataset was tried first. That deletion degrades just like the ones it replaces, so it saved only 12%. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

hannes-ucsc · 2026-10-22T00:28:56Z

(Posted by Claude Code)

Security design review

Every box is checked: this PR affects none of the listed categories, and no vulnerability was identified. The notes below are informational, recording what was examined in reaching that conclusion.

Informational: the change to _merge_links affects only the order of the merged links array, not its membership. Stitching is confined to the root bundle's own snapshot, because _stitch_bundles passes the root bundle's source to _find_upstream_bundles, so a deterministic merge cannot draw links from a snapshot outside the requester's entitlement.

Informational: replica coordinates comprise entity type, entity ID and content hash, and a replica carries its bundle's source. Collapsing the duplicate links replicas therefore leaves both the entity ID and the source unchanged, and the indexer already consolidated the hub_ids of those duplicates. No requester gains access to a replica that was previously out of reach. This rests on reading ReplicaCoordinates and Transformer._replica; the service-side source filter was not traced and nothing was exercised at runtime.

Informational: the six UUIDs removed from the canned bundle's stitched set are all manifest entry UUIDs, none of them an entity document_id, so they never matched in Bundle.not_stitched. That change is confined to a fixture.

Informational: the new fixture rows are synthetic, reference the mock source tdr:bigquery:gcp:test_hca_project:hca_snapshot and a drs://mock_tdr.lan/… URI, and contain no credentials, real endpoints or personal data.

Informational: the BigQuery emulator container is now created per test rather than per test class, and killed in tearDown rather than tearDownClass. The port mapping is unchanged — 127.0.0.1 on a developer machine, the bridge IP for a sibling container in CI — and the listener's lifetime is shorter than before. The emulator image is already listed in docker_images.json.

achave11-ucsc requested a review from hannes-ucsc as a code owner October 20, 2026 13:38

achave11-ucsc self-assigned this Oct 20, 2026

achave11-ucsc marked this pull request as draft October 20, 2026 13:39

achave11-ucsc added reindex:dev [process] PR requires reindexing dev reindex:prod [process] PR requires reindexing prod labels Oct 20, 2026

achave11-ucsc force-pushed the issues/achave11-ucsc/8393-8259-8294-stitched-links-order branch from 7c28fc8 to 5399925 Compare October 20, 2026 14:00

achave11-ucsc added reindex:partial [process] PR allows for (de)indexing a specific source or catalog and removed reindex:dev [process] PR requires reindexing dev labels Oct 20, 2026

achave11-ucsc commented Oct 20, 2026

View reviewed changes

achave11-ucsc force-pushed the issues/achave11-ucsc/8393-8259-8294-stitched-links-order branch from 7c28fc8 to 54f6951 Compare October 20, 2026 20:45

achave11-ucsc assigned hannes-ucsc Oct 20, 2026

achave11-ucsc marked this pull request as ready for review October 20, 2026 21:00

hannes-ucsc mentioned this pull request Oct 21, 2026

Canned DSS bundles have manifest entry UUIDs in their stitched sets #8423

Open

hannes-ucsc and others added 7 commits October 21, 2026 09:06

hannes-ucsc force-pushed the issues/achave11-ucsc/8393-8259-8294-stitched-links-order branch from dc5a004 to 43bf9f7 Compare October 21, 2026 21:12

hannes-ucsc removed the reindex:partial [process] PR allows for (de)indexing a specific source or catalog label Oct 22, 2026

hannes-ucsc approved these changes Oct 22, 2026

View reviewed changes

hannes-ucsc added the reindex:dev [process] PR requires reindexing dev label Oct 22, 2026

hannes-ucsc added 0 reviews [process] Lead didn't request any changes sandbox [process] Resolution is being verified in sandbox deployment labels Oct 22, 2026

hannes-ucsc merged commit 8842489 into develop Oct 22, 2026
10 checks passed

hannes-ucsc mentioned this pull request Oct 22, 2026

PUT /fetch/manifest/files returns 500 once 13 executions of a manifest have failed #8399

Open

hannes-ucsc deleted the issues/achave11-ucsc/8393-8259-8294-stitched-links-order branch October 22, 2026 03:51

hannes-ucsc unassigned achave11-ucsc and hannes-ucsc Oct 22, 2026

achave11-ucsc linked an issue Oct 22, 2026 that may be closed by this pull request

Canned DSS bundles have manifest entry UUIDs in their stitched sets #8423

Open

Navigation Menu

Repository navigation

Fix: Stitched HCA bundles are indexed as many links replicas that differ only in link order (#8393, #8259, #8294) - #8418

Fix: Stitched HCA bundles are indexed as many links replicas that differ only in link order (#8393, #8259, #8294)#8418
hannes-ucsc merged 7 commits into
developfrom
issues/achave11-ucsc/8393-8259-8294-stitched-links-order

achave11-ucsc commented Oct 20, 2026 •

edited by hannes-ucsc

Loading

Uh oh!

codecov Bot commented Oct 20, 2026 •

edited

Loading

Uh oh!

coveralls commented Oct 20, 2026 •

edited

Loading

Uh oh!

achave11-ucsc Oct 20, 2026 •

edited

Loading

Uh oh!

hannes-ucsc commented Oct 22, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

achave11-ucsc commented Oct 20, 2026 • edited by hannes-ucsc Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Author

Author (partiality)

Author (reindex)

Author (mirror)

Author (API changes)

Author (upgrading deployments)

Author (hotfixes)

Author (before every review)

Peer reviewer (after approval)

System administrator (after approval)

Operator

Operator (deploy .shared and .gitlab components)

System administrator (post-deploy of .gitlab component)

Operator (deploy runner image)

Operator (sandbox build)

Operator (merge the branch)

Operator (main build)

Operator (reindex)

Operator (mirroring)

Operator

Shorthand for review comments

Uh oh!

codecov Bot commented Oct 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coveralls commented Oct 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

achave11-ucsc Oct 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hannes-ucsc commented Oct 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Security design review

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

achave11-ucsc commented Oct 20, 2026 •

edited by hannes-ucsc

Loading

Operator (deploy `.shared` and `.gitlab` components)

System administrator (post-deploy of `.gitlab` component)

codecov Bot commented Oct 20, 2026 •

edited

Loading

coveralls commented Oct 20, 2026 •

edited

Loading

achave11-ucsc Oct 20, 2026 •

edited

Loading

hannes-ucsc commented Oct 22, 2026 •

edited

Loading