|
| 1 | +--- |
| 2 | +summary: Folded the tag-selection chain into _select_latest_semver_tag, which carries the parsed Version to _compute_new_version; next_version finalizes SemVer-form prerelease baselines. |
| 3 | +--- |
| 4 | + |
| 5 | +# Design: Deepen the semver-tag selection chain |
| 6 | + |
| 7 | +## Summary |
| 8 | + |
| 9 | +The "pick the bump baseline and bump it" step in `semvertag/_use_case.py` is four |
| 10 | +shallow pure helpers: `_try_parse_semver` → `_parse_semver_tags` → |
| 11 | +`_pick_latest_semver_tag` → `_compute_new_version`. Each is trivially correct in |
| 12 | +isolation, but the *composed* behavior — skip-unparseable, sort by precedence, |
| 13 | +pick-max, then bump — has emergent semantics that no helper test captures, and the |
| 14 | +winning tag is **parsed twice** (the selector discards the parsed `Version`, then |
| 15 | +`_compute_new_version` re-parses it). This change folds the three parse-helpers |
| 16 | +into one `_select_latest_semver_tag(tags) -> tuple[Tag, semver.Version] | None` |
| 17 | +that carries the parsed `Version` through, and switches the bump arithmetic from |
| 18 | +`bump_*` to `Version.next_version` so a SemVer-form prerelease baseline finalizes |
| 19 | +(`1.0.0-rc.1` + patch → `1.0.0`, not `1.0.1`). One interface becomes the real test |
| 20 | +surface; the emergent edges get explicit tests. See |
| 21 | +[`decisions/2026-06-26-semver-form-tags-only.md`](../../decisions/2026-06-26-semver-form-tags-only.md). |
| 22 | + |
| 23 | +## Motivation |
| 24 | + |
| 25 | +`semvertag/_use_case.py:54-85`: |
| 26 | + |
| 27 | +- **Parse-twice.** `_pick_latest_semver_tag` parses every tag into a `Version`, |
| 28 | + sorts, returns only the `Tag` — discarding the `Version`. `_compute_new_version` |
| 29 | + then re-parses `last_tag.name`. |
| 30 | +- **Untested emergent surface.** PEP 440 prereleases (`0.9.0rc1`) and `v`-prefixed |
| 31 | + tags (`v0` — a real tag in this repo) are silently skipped by strict |
| 32 | + `Version.parse`; build-metadata ties are input-order-dependent. None of this is |
| 33 | + tested at the seam where it lives — the only non-semver test uses |
| 34 | + `release-2024-Q1`/`latest`. |
| 35 | +- **Latent prerelease bug.** `bump_patch` on a SemVer-form prerelease baseline |
| 36 | + (`1.0.0-rc.1`) jumps to `1.0.1` instead of finalizing to `1.0.0`. |
| 37 | + |
| 38 | +The four helpers are the textbook "extracted for testability, but the real bug is |
| 39 | +in how they're composed" shape — low locality. Deletion test: folding them |
| 40 | +concentrates the selection logic in one interface rather than scattering a |
| 41 | +four-hop chain. |
| 42 | + |
| 43 | +## Non-goals |
| 44 | + |
| 45 | +- **No PEP 440 recognition** and **no `v`-prefix recognition** — both decided in |
| 46 | + `decisions/2026-06-26-semver-form-tags-only.md` (rejected / deferred). Selection |
| 47 | + stays SemVer-form only. |
| 48 | +- No change to the `Outcome` variants, the providers, strategies, output, or DI. |
| 49 | +- No change for *stable* baselines **without build metadata**: `next_version(part)` |
| 50 | + equals `bump_*` there, so every existing bare-semver tag behaves identically. The |
| 51 | + selector strips build metadata (precedence-irrelevant; semvertag never emits it), |
| 52 | + so the carried `Version` is always build-free and `next_version` is never tripped |
| 53 | + by a `1.0.0+build`-style tag. |
| 54 | + |
| 55 | +## Design |
| 56 | + |
| 57 | +### 1. One selector carrying the parsed `Version` |
| 58 | + |
| 59 | +Replace `_try_parse_semver` / `_parse_semver_tags` / `_pick_latest_semver_tag` |
| 60 | +with: |
| 61 | + |
| 62 | +```python |
| 63 | +def _select_latest_semver_tag(tags: list[Tag]) -> tuple[Tag, semver.Version] | None: |
| 64 | + parsed: list[tuple[semver.Version, Tag]] = [] |
| 65 | + for tag in tags: |
| 66 | + try: |
| 67 | + version = semver.Version.parse(tag.name).replace(build=None) |
| 68 | + except ValueError: |
| 69 | + continue |
| 70 | + parsed.append((version, tag)) |
| 71 | + if not parsed: |
| 72 | + return None |
| 73 | + parsed.sort(key=lambda item: item[0]) |
| 74 | + version, tag = parsed[-1] |
| 75 | + return tag, version |
| 76 | +``` |
| 77 | + |
| 78 | +`sorted(...)[-1]` (not `max`) preserves the current **last-equal-wins** tie order |
| 79 | +for versions that compare equal (build metadata is ignored in precedence). The |
| 80 | +`.replace(build=None)` strips build metadata from the carried `Version` so that |
| 81 | +`next_version` never treats a `1.0.0+build`-style baseline as already-finalized |
| 82 | +and skips the bump. semvertag never emits build metadata; stripping it is |
| 83 | +precedence-neutral. |
| 84 | + |
| 85 | +### 2. Bump via `next_version`, on the carried `Version` |
| 86 | + |
| 87 | +```python |
| 88 | +_BUMP_PARTS: typing.Final[dict[Bump, str]] = {Bump.MAJOR: "major", Bump.MINOR: "minor", Bump.PATCH: "patch"} |
| 89 | + |
| 90 | +def _compute_new_version(version: semver.Version, bump: Bump) -> str: |
| 91 | + return str(version.next_version(_BUMP_PARTS[bump])) |
| 92 | +``` |
| 93 | + |
| 94 | +It takes the `Version` from the selector tuple — the winning tag is never parsed |
| 95 | +twice. `next_version` is behavior-preserving on stable baselines and finalizes |
| 96 | +SemVer-form prerelease baselines. |
| 97 | + |
| 98 | +### 3. Use-case wiring |
| 99 | + |
| 100 | +```python |
| 101 | +tags = self.provider.list_tags() |
| 102 | +selected = _select_latest_semver_tag(tags) |
| 103 | +if selected is None: |
| 104 | + return self._emit(output, NoTags(commit=commit.sha)) |
| 105 | +latest_tag, latest_version = selected |
| 106 | +if latest_tag.commit_sha == commit.sha: |
| 107 | + return self._emit(output, AlreadyTagged(tag=latest_tag.name, commit=commit.sha)) |
| 108 | +... |
| 109 | +new_version = _compute_new_version(latest_version, bump) |
| 110 | +``` |
| 111 | + |
| 112 | +Same `NoTags` / `AlreadyTagged` / no-bump logic; only the carried `Version` is new. |
| 113 | + |
| 114 | +## Testing |
| 115 | + |
| 116 | +TDD. Direct helper tests in `tests/unit/test_use_case.py` (helpers stay in |
| 117 | +`_use_case.py`): |
| 118 | + |
| 119 | +- `_select_latest_semver_tag`: empty → `None`; all-unparseable |
| 120 | + (`release-2024-Q1`, `latest`, `v0`) → `None`; PEP 440 skip (`[0.8.1, 0.9.0rc1]` |
| 121 | + → `0.8.1`); SemVer-form prerelease participates/orders (`[1.0.0-rc.1, 0.9.0]` → |
| 122 | + `1.0.0-rc.1`); tie last-wins (`[1.0.0+a (x), 1.0.0+b (y)]` → `1.0.0+b`); returns |
| 123 | + the parsed `Version` alongside the `Tag`. |
| 124 | +- `_compute_new_version`: finalize (`Version.parse("1.0.0-rc.1")`, `Bump.PATCH`) → |
| 125 | + `"1.0.0"`; stable cases unchanged. |
| 126 | + |
| 127 | +Keep all existing use-case integration tests (behavior-preservation proof; they |
| 128 | +stay green). Gates: `just test` (100% branch), `just lint-ci`, `just docs-build`. |
| 129 | + |
| 130 | +## Risk |
| 131 | + |
| 132 | +- **`next_version` behavior change (medium × low).** Mitigated: it equals `bump_*` |
| 133 | + on every stable baseline (so all existing tests pass unchanged), and differs only |
| 134 | + by finalizing prerelease baselines, which is the intended fix. The existing |
| 135 | + parametrized bump test (`1.4.2` → major/minor/patch) is the guardrail. |
| 136 | +- **Tie-order regression (low × low).** `sorted(...)[-1]` preserves last-equal-wins; |
| 137 | + a new test pins it. |
| 138 | +- **Coverage (low × low).** The folded selector's branches (parse ok/skip, empty, |
| 139 | + sort) and the `_BUMP_PARTS` lookups are covered by the direct tests plus the |
| 140 | + existing suite. |
| 141 | +- **`architecture/cli.md` drift (low × low).** The Use-case section names |
| 142 | + `_pick_latest_semver_tag` and `bump_*`; promote it in the same PR. |
0 commit comments