Skip to content

fix: emit commit_authors in commit order, not set order - #13

Draft
gkrimer wants to merge 1 commit into
mainfrom
fix/commit-authors-order
Draft

fix: emit commit_authors in commit order, not set order#13
gkrimer wants to merge 1 commit into
mainfrom
fix/commit-authors-order

Conversation

@gkrimer

@gkrimer gkrimer commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Summary

Rebased onto main now that #12 is merged — was stacked on it, since main had no pytest and I didn't want to ship an untested behavior change to the deploy path.

#11 removed sorted() so authors would "appear in same order as the commits" — but commit_authors stayed a set, so ",".join(...) emits hash order, which Python randomizes per process. The intent was never achieved:

❯ for seed in 0 1 2 3; do echo "--- PYTHONHASHSEED=$seed ---"; \
    PYTHONHASHSEED=$seed PYTHONPATH=. uv run python order_check.py; done
--- PYTHONHASHSEED=0 ---
new (list): gkrimer,celinefb,instrumentl707,gsinkin-instrumentl
old (set):  celinefb,instrumentl707,gsinkin-instrumentl,gkrimer
--- PYTHONHASHSEED=1 ---
new (list): gkrimer,celinefb,instrumentl707,gsinkin-instrumentl
old (set):  gkrimer,instrumentl707,gsinkin-instrumentl,celinefb
--- PYTHONHASHSEED=2 ---
new (list): gkrimer,celinefb,instrumentl707,gsinkin-instrumentl
old (set):  gkrimer,instrumentl707,gsinkin-instrumentl,celinefb
--- PYTHONHASHSEED=3 ---
new (list): gkrimer,celinefb,instrumentl707,gsinkin-instrumentl
old (set):  gkrimer,instrumentl707,celinefb,gsinkin-instrumentl

Extracts the GitHub lookup into github_logins, returning a list ordered by author_to_sha insertion — i.e. commit order, newest first, since enumerate_changes walks rev-list. Duplicate logins are dropped keeping the first position, because distinct author emails can map to one GitHub account.

  • create_tag.py: commit_authors set → ordered list via github_logins
  • create_tag.py: drop write-only change_authors, dead since #10 — assigned and .add()ed, never read, so ruff's F841 doesn't fire on it
  • action.yml: the commit_authors description said "first page of author usernames", stale since fix: separate deploying actor from commit authors #10 replaced comp.commits.get_page(0) with per-author get_commit; now documents the ordering contract instead
  • create_tag_test.py: 4 tests over order, dedupe, lookup failure, unlinked email

Blast radius

commit_authors has exactly two consumers, both in auto-deploy-to-gke.yml: the Cloud Deploy release description, and the github-author-names annotation. Grepping instrumentl, gke-scripts, and terraform-gcp finds nothing reading that annotation back, so this is a determinism/readability fix, not a semantic one. No consumer changes needed.

Test plan

  • ruff check / ruff format --check / pyright / pytest (13 tests) / ./smoke-test.sh pass locally
  • Ordering is stable across PYTHONHASHSEED values (above); old set is not
  • CI green on the rebase — 13 tests
  • Next real prod deploy: github-author-names annotation lists authors newest-commit-first

🤖 Generated with Claude Code

Base automatically changed from chore/modernize-python-tooling to main July 28, 2026 00:19
#11 removed `sorted()` so authors would "appear in same order as the commits",
but `commit_authors` stayed a `set`, so `",".join(...)` emits hash order --
which Python randomizes per process. The stated intent was never achieved:

  ❯ for seed in 0 1 2 3; do PYTHONHASHSEED=$seed python order_check.py; done
  --- PYTHONHASHSEED=0 ---
  new (list): gkrimer,celinefb,instrumentl707,gsinkin-instrumentl
  old (set):  celinefb,instrumentl707,gsinkin-instrumentl,gkrimer
  --- PYTHONHASHSEED=1 ---
  new (list): gkrimer,celinefb,instrumentl707,gsinkin-instrumentl
  old (set):  gkrimer,instrumentl707,gsinkin-instrumentl,celinefb
  --- PYTHONHASHSEED=3 ---
  new (list): gkrimer,celinefb,instrumentl707,gsinkin-instrumentl
  old (set):  gkrimer,instrumentl707,celinefb,gsinkin-instrumentl

Extracts the GitHub lookup into `github_logins`, which returns a list ordered by
`author_to_sha` insertion (i.e. commit order, newest first) and drops duplicate
logins while keeping the first position -- distinct author emails can map to one
GitHub account.

- create_tag.py: `commit_authors` set -> ordered list via `github_logins`
- create_tag.py: drop write-only `change_authors`, dead since #10
- action.yml: `commit_authors` description said "first page", stale since #10
  removed the paginated lookup; now documents the ordering contract
- create_tag_test.py: 4 tests over order, dedupe, lookup failure, unlinked email

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gkrimer
gkrimer force-pushed the fix/commit-authors-order branch from b6c2169 to cb38b08 Compare July 28, 2026 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant