Repository navigation
Conversation
There was a problem hiding this comment.
All reported issues were addressed
Shadow auto-approve: would not auto-approve because issues were found.
View guided diff | Re-trigger cubic
There was a problem hiding this comment.
Review completed against the latest diff
Shadow auto-approve: would not auto-approve because issues were found.
View guided diff | Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Shadow auto-approve: would not auto-approve because issues were found.
View guided diff | Re-trigger cubic
cragwolfe
left a comment
There was a problem hiding this comment.
Reviewed b64f3236f776b56e30420ee930b637efcc787fd5 with GPT Pro. One actionable issue remains.
P2 — A later retry can cancel the only pending development bump (.github/workflows/post-release-dev-bump.yml:16–18). All release events and manual retries share one concurrency group. cancel-in-progress: false protects the running workflow, but the default group permits only one pending run; a later arrival replaces it. For example, while an old 0.27.34 retry is running, publishing 0.27.35 queues its valid bump. A second old retry then cancels that pending release run. Both old retries fail the main-version precondition, leaving no 0.27.36-dev PR and ordinary PRs blocked by check-version. An ignored-tag event can likewise displace the valid pending run before filtering.
Preserve pending work while retaining serialization, for example with queue: max, or have surviving runs reconcile the current final version instead of only their triggering tag. GitHub documents the default replacement policy and larger queues. Verify that an old retry running, a current release pending, and another old retry arriving still allow the current release to create or reuse its bump PR; retain same-target serialization and post-merge no-op behavior.
The existing prefix comment does not establish a supported-publishing regression: release.yml accepts numeric and v-prefixed package versions; broader historical tag parsing in the version guard serves reservation and ordering. The test-assertion and duplicated-sed comments do not demonstrate a current production failure.
All required exact-head checks pass, including version, lint, unit, ingest, and Docker. No local tests, reproductions, release/workflow executions, or bump PR creation were performed.
(authored by codex)
Key the workflow's concurrency group by release tag, so a retry or ignored tag can no longer replace the pending run for a real release. Accept unstructured_-prefixed tags, assert the complete bump decision in tests, and clean up the test's temporary repository. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Addressed the P2 concurrency finding from the GPT Pro review in d9a07b5. The concurrency group is now keyed by release tag: Dropping cross-tag serialization is safe. A run only acts when In the reviewer's scenario (old retry running, Same commit, from cubic: |
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Shadow auto-approve: would not auto-approve because issues were found.
View guided diff | Re-trigger cubic
cragwolfe
left a comment
There was a problem hiding this comment.
Reviewed the complete final changes and surrounding release code with GPT Pro at d9a07b5f7f551538b06afc76c03509105ac87831, against actual main base 803b9005f9f2406b839e9bbec603f40e6d322693. Two P2 issues remain.
P2 — A case-variant retry can still replace valid pending release work (.github/workflows/post-release-dev-bump.yml:19–21; scripts/post-release-dev-bump.sh:10–15). GitHub concurrency groups ignore case and replace the pending run by default, but the planner strips only lowercase prefixes. With main at 0.27.35, a manual V0.27.35 retry can occupy the same effective group as a published v0.27.35 release. If the valid release is pending and another uppercase retry arrives, it replaces the valid run. Both uppercase retries then skip; neither needs a published release because verification runs only for action=bump. No development PR is created, leaving ordinary PRs blocked. Preserve valid pending work and serialize the canonical bump target, with checkout/planning inside the serialized operation. A shared group with queue: max is one option; restoring the old single-pending global group is not sufficient. Cover this three-arrival case and the original old-retry/current-release case. The current test only pins the raw group expression.
P2 — Retrying after a supported dotted development bump reports a false failure (scripts/post-release-dev-bump.sh:28–33). The no-op branch accepts only -dev, although the shared parser accepts .dev and CONTRIBUTING documents .dev0 equivalence. After a maintainer recovers manually to 0.27.36.dev0, retrying release 0.27.35 fails with Unexpected main version instead of returning action=noop. This does not corrupt the version or block ordinary merges, but incorrectly reports a completed recovery as a release-automation failure. Use the shared parser to determine development status; cover dotted bare/numbered suffixes and retain rejection of older development and unexpected final versions.
The precise old-version retry scenario in the prior review is fixed by this commit. Author comment #6424512917 and the prefix, complete-output assertion, and temporary-directory changes were considered. Accepted aliases also use different raw groups while targeting one branch; any replacement serialization should cover that shared target. I am not claiming a demonstrated version rollback or loss of both alias requests.
Human review required separately: accept the existing token's automatic version-PR and squash auto-merge role, confirm the intended independent approval/check rules apply to its identity, and decide whether GitHub Release publication is sufficient to start the next development version before package upload succeeds. This changes the maintainer release process; technical corrections and CI do not settle that policy.
Exact-head CI currently has 52 successful checks and 11 skipped checks; CI, CodeQL, Security, and Partition Benchmark workflows succeeded. No local tests, reproductions, workflow/release execution, source edits, or configuration changes were performed. uv.lock and the unchanged CHANGELOG.md were excluded from the Pro bundle; the package version is unchanged versus main. This COMMENT is not approval or a merge-readiness claim.
(authored by codex)
There was a problem hiding this comment.
0 issues found across 5 files (changes from recent commits).
Shadow auto-approve: would not auto-approve. This PR does not meet the repository auto-approval settings.
View guided diff | Re-trigger cubic
cragwolfe
left a comment
There was a problem hiding this comment.
Full GPT Pro production review of 057d6d6d5bce493a7b5f6a5490ff8e7a2e63659b against main 7bda7d82887a78bde8c19caf6574514d62a82574: technically clean. Both previous P2 findings are resolved.
- A shared
queue: maxserializes all bump writers and preserves accepted pending work across aliases, case variants, old retries, and ignored tags. Checkout and planning occur after queue admission. The documented limit is 100 pending runs; later arrivals at capacity must be retried. - Completed-bump retries use the shared version parser and ordering, including dotted development versions. Earlier development versions, malformed versions, and unexpected final versions still fail without mutation.
Focused regression tests failed before the fixes and passed afterward. The queue tests also fail when restored to single-pending behavior. Exact-head CI and required checks passed, including version guard, lint, unit, ingest, Docker, Security, CodeQL, and the partition benchmark.
HUMAN REVIEW REQUIRED remains: a maintainer must accept the token's automatic version-PR/auto-merge role, verify independent approval and merge rules for that identity, and decide whether GitHub Release publication and the automatic next-patch convention are the intended release policy. This technical result does not accept those decisions or authorize integration. No merge, auto-merge activation, release, or secret change was performed.
(authored by codex)
Release PR: changes `unstructured/__version__.py` from `0.27.35-dev` to `0.27.35`, per CONTRIBUTING.md release-day step 1. No changelog edit (CHANGELOG.md is frozen). This is the first release since the post-release dev-bump automation landed (#4590). After the `0.27.35` GitHub Release is published, the workflow should open a `chore: start 0.27.36-dev after 0.27.35 release` PR automatically.
TL;DR
After a package release is published, open a PR to start the next patch development version. This removes the manual PR creation step that blocks ordinary PRs while
mainholds a final version. A maintainer still approves the bump.Trigger and safety
The new workflow handles
release: publishedand manual retries with ataginput. It accepts final numeric package tags, strips an optionalv, and skips development, OpenCV, and docs tags. It changes onlyunstructured/__version__.pyin the generated PR, for example0.27.35to0.27.36-dev.It acts only when
mainstill holds the released version. Ifmainalready holds a development version at least as new as the target, the run is a successful no-op; any other version fails without a change. Before acting, it also verifies that the supplied tag identifies a published final GitHub Release. Serialized runs and a fixed branch per target version let retries update the same PR. The existing version guard already covers a final version to its next-patch-dev; the new decision tests run alongside that guard in CI.Token, approval, and recovery
PR creation and squash auto-merge use the existing
GH_CREATE_PR_TOKEN. PRs opened byGITHUB_TOKENdo not trigger the required workflows, so it cannot be used for creation. The default token has onlycontents: readpermission. Actions follow the repository's version-tag pinning style.The token owner authors the bump PR. Another maintainer must approve it. Auto-merge requests entry into the merge queue after approval and required checks. If auto-merge is not permitted, the workflow warns and a maintainer must enqueue it. A missing or expired token, or failed PR creation, produces an error directing maintainers to the manual bump in CONTRIBUTING.md.
After merge, a safe smoke test is to run
workflow_dispatchwith the latest package tag whilemainalready holds the next-dev: the run should report a no-op and create no PR. This implementation PR has no version or changelog change, and does not change publication workflows.Validation
Offline version decisions and guard
Tests cover version computation, optional
v, ignored tags, bare and numbered development versions, and unexpected main versions. The tests also cover the actual version-file format and release-query failure. The existing guard tests confirm the generated bump passescheck-version.Shell checks
shellcheck scripts/check-new-release-version.sh scripts/post-release-dev-bump.sh scripts/test-post-release-dev-bump.sh shfmt -i 2 -d scripts/check-new-release-version.sh scripts/post-release-dev-bump.sh scripts/test-post-release-dev-bump.sh # Both passed with no output.Reviews
b64f3236f776b56e30420ee930b637efcc787fd5. The completed browser verdict was recovered after a capture timeout.At that head,
check-version, ShellCheck, shfmt, lint, CodeQL, and the completed test jobs passed. No failures were reported; unit, PDF/image dependency, and Docker jobs were still running at handoff. Long jobs were not awaited. Actionlint was not installed, and installation failed on a TLS timeout. No live release or workflow dispatch was run.authored by codex
Repair result (2026-10-26)
Both P2 findings are fixed in
057d6d6d5bce493a7b5f6a5490ff8e7a2e63659b. All tag aliases now share one queue withqueue: max, and each run checks out and plans against currentmainafter waiting. Accepted pending work survives ignored tags, case variants, and stale retries. GitHub keeps up to 100 pending runs and cancels later arrivals at capacity; maintainers can retry after the queue drains. Completed-bump retries use the shared version parser and accept supported dotted development versions.The queue and dotted-development regression tests fail before the fixes and pass afterward. The queue tests model GitHub's documented scheduling contract; no live release workflow was invoked.
Focused validation
ShellCheck, shfmt, Bash syntax, and YAML parsing passed. Actionlint 1.7.12 does not recognize the documented
queueproperty and reports the existing custom runner labels and the PR action's legacy runtime declaration; these diagnostics were assessed in the full review.A fresh full-context GPT Pro review at this head against main
7bda7d82887a78bde8c19caf6574514d62a82574is technically clean, with both P2 findings resolved and no further code changes requested. Exact-head required checks passed; CI, CodeQL, Security, and Partition Benchmark succeeded. The review excludesuv.lockandCHANGELOG.md; neither is changed by this PR.Human release-policy review remains required. Maintainers must decide the token's automatic version-PR and auto-merge role, confirm independent approval and merge rules for that identity, and accept the GitHub Release trigger and next-patch development convention. The technical result does not accept those decisions. No merge, auto-merge activation, release, or secret change was performed.
(authored by codex)