SRE-985: Remove the vestigial head-SHA checkout pin from the unit-tests job - #9439
Conversation
The `setup` job computes the affected-package matrix on a pull request's merge ref, while `unit-tests` pinned its own checkout to the head SHA. When a PR's head predates a package that has since landed on `main`, the matrix names a package that is absent from the head tree and the job fails at its "Prune repository" step. The pin arrived with #8034 for DeepSource's coverage action; #8041 removed DeepSource and left the pin behind. Nothing reads it now, so the job joins the rest of the file on the merge ref.
Both `Checkout` steps guarded by `github.event_name != 'pull_request'` set `ref` to `github.event.pull_request.head.sha`. bench.yml runs only on `pull_request` and `push`, so on the events that reach these steps the payload carries no pull request and the expression is empty; actions/checkout treats an empty `ref` as unset and checks out the triggering ref regardless. The deliberate base/head checkout pairs that drive A/B benchmarking on pull requests are untouched.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9439 +/- ##
==========================================
+ Coverage 60.66% 61.00% +0.33%
==========================================
Files 1439 1446 +7
Lines 143119 144453 +1334
Branches 6654 6687 +33
==========================================
+ Hits 86817 88117 +1300
- Misses 55211 55232 +21
- Partials 1091 1104 +13 Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Merging this PR will degrade performance by 15.38%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ❌ | bit_matrix/dense/iter_row[64] |
140.8 ns | 170 ns | -17.16% |
| ❌ | bit_matrix/dense/iter_row[200] |
185.8 ns | 215 ns | -13.57% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing claude/sre-985-drop-unit-tests-head-sha-pin (0bcf754) with main (59468b1)1
Footnotes
PR SummaryLow Risk Overview Also removes the same vestigial Reviewed by Cursor Bugbot for commit 0bcf754. Bugbot is set up for automated code reviews on this repo. Configure here. |
Benchmark results
|
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 2002 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 1002 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: high, policies: 3314 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: medium, policies: 1527 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 2078 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 1033 | Flame Graph |
policy_resolution_medium
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 102 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 52 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: high, policies: 269 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: medium, policies: 108 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 133 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 63 | Flame Graph |
policy_resolution_none
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 2 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 2 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 8 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 3 | Flame Graph |
policy_resolution_small
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 52 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 26 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: high, policies: 94 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: medium, policies: 27 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 66 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 29 | Flame Graph |
read_scaling_complete
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_id;one_depth | 1 entities | Flame Graph | |
| entity_by_id;one_depth | 10 entities | Flame Graph | |
| entity_by_id;one_depth | 25 entities | Flame Graph | |
| entity_by_id;one_depth | 5 entities | Flame Graph | |
| entity_by_id;one_depth | 50 entities | Flame Graph | |
| entity_by_id;two_depth | 1 entities | Flame Graph | |
| entity_by_id;two_depth | 10 entities | Flame Graph | |
| entity_by_id;two_depth | 25 entities | Flame Graph | |
| entity_by_id;two_depth | 5 entities | Flame Graph | |
| entity_by_id;two_depth | 50 entities | Flame Graph | |
| entity_by_id;zero_depth | 1 entities | Flame Graph | |
| entity_by_id;zero_depth | 10 entities | Flame Graph | |
| entity_by_id;zero_depth | 25 entities | Flame Graph | |
| entity_by_id;zero_depth | 5 entities | Flame Graph | |
| entity_by_id;zero_depth | 50 entities | Flame Graph |
read_scaling_linkless
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_id | 1 entities | Flame Graph | |
| entity_by_id | 10 entities | Flame Graph | |
| entity_by_id | 100 entities | Flame Graph | |
| entity_by_id | 1000 entities | Flame Graph | |
| entity_by_id | 10000 entities | Flame Graph |
representative_read_entity
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/block/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/book/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/building/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/organization/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/page/v/2
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/person/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/playlist/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/song/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/uk-address/v/1
|
Flame Graph |
representative_read_entity_type
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| get_entity_type_by_id | Account ID: bf5a9ef5-dc3b-43cf-a291-6210c0321eba
|
Flame Graph |
representative_read_multiple_entities
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_property | traversal_paths=0 | 0 | |
| entity_by_property | traversal_paths=255 | 1,resolve_depths=inherit:1;values:255;properties:255;links:127;link_dests:126;type:true | |
| entity_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:0;links:0;link_dests:0;type:false | |
| entity_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:0;links:1;link_dests:0;type:true | |
| entity_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:2;links:1;link_dests:0;type:true | |
| entity_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:2;properties:2;links:1;link_dests:0;type:true | |
| link_by_source_by_property | traversal_paths=0 | 0 | |
| link_by_source_by_property | traversal_paths=255 | 1,resolve_depths=inherit:1;values:255;properties:255;links:127;link_dests:126;type:true | |
| link_by_source_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:0;links:0;link_dests:0;type:false | |
| link_by_source_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:0;links:1;link_dests:0;type:true | |
| link_by_source_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:2;links:1;link_dests:0;type:true | |
| link_by_source_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:2;properties:2;links:1;link_dests:0;type:true |
scenarios
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| full_test | query-limited | Flame Graph | |
| full_test | query-unlimited | Flame Graph | |
| linked_queries | query-limited | Flame Graph | |
| linked_queries | query-unlimited | Flame Graph |
Requested by Tim Diekmann · Slack thread
🌟 What is the purpose of this PR?
Before: the
setupjob intest.ymlworks out which packages a pull request affects by looking at the merge ref — the PR's changes as they would land onmain. Theunit-testsjob then pinned its own checkout to the PR head instead. So the two jobs disagreed about which tree they were talking about. If a PR's head is older than a package that has since landed onmain,setuphandsunit-testsa matrix entry for a package that does not exist in the head tree, and the job dies at its "Prune repository" step. The change under test is fine; the workflow put it in an impossible position.After:
unit-testschecks out the same merge ref that produced its matrix, so the matrix always names packages that are actually present. This is what every other job already does —setup,integration-testsandpublish-rustin this file, and the whole oflint.yml,codspeed.ymlanddeploy.yml. The pin was the only one of its kind intest.yml.How: the fix is to delete the two-line
with: ref:block from theunit-testscheckout. The pin was added by #8034 (SRE-133) because DeepSource's coverage action wanted the head SHA. #8041 removed DeepSource five days later and left the pin behind. Nothing depends on it today: DeepSource is gone from the repository entirely, and the Codecov upload steps take no SHA input — the identical steps already run unpinned inintegration-tests, which is the direct evidence that Codecov does not need it.🔗 Related links
HEAD^. Not touched here.🔍 What does this change?
.github/workflows/test.yml: dropref: ${{ github.event.pull_request.head.sha }}from theunit-testscheckout. This is the change the ticket asks for..github/workflows/bench.yml: a separable tidy-up, in its own second commit so it can be dropped without touching the first. A sweep of everything under.github/for head-ref checkout pins turned up four more inbench.yml. Two pairs (base.shathenhead.sha, both gatedif: github.event_name == 'pull_request') are the deliberate A/B benchmarking checkouts and are left exactly as they are. The other two are dead: they sit onCheckoutsteps gatedif: github.event_name != 'pull_request', andbench.ymltriggers only onpull_requestandpush, so whenever those steps run there is no pull request in the payload and the expression evaluates to the empty string.actions/checkouttreats an emptyrefas unset and checks out the triggering ref anyway, so the inputs have never done anything. Removing them changes no behaviour.Validation:
actionlintv1.7.12 (the versionhousekeeping.ymlpins) reports exactly the same findings on this branch as onmain— 29, all of them the$/self-repository syntax that this actionlint version predates. Both files still parse as YAML and the job and step structure is unchanged apart from the deletions above.Pre-Merge Checklist 🚀
🚢 Has this modified a publishable library?
This PR:
📜 Does this require a change to the docs?
The changes in this PR:
🕸️ Does this require a change to the Turbo Graph?
The changes in this PR:
None. Both changes are deletions of inputs that nothing reads.
🛡 What tests cover this?
CI on this PR is the test.
unit-testsrunning green proves the job still checks out a usable tree without the pin.❓ How to test this?
Test / Unitjobs on this PR — they now run on the merge ref.main, and confirm onmainthatunit-testsfails at "Prune repository" whilesetuplists the new package.