Compare commits
23
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
f70df9f827 | ||
|
|
4ce47397a9 | ||
|
|
721154847e | ||
|
|
633d4f591f | ||
|
|
f5dd4462de | ||
|
|
31190657d8 | ||
|
|
ecfa056d4d | ||
|
|
f367eeaa9d | ||
|
|
270ad7a71b | ||
|
|
17212e9eb4 | ||
|
|
8f4b76a638 | ||
|
|
aeb8781c4e | ||
|
|
439c8625d5 | ||
|
|
88508b536b | ||
|
|
1d67c160b2 | ||
|
|
90bb3538c6 | ||
|
|
a687ef439c | ||
|
|
eaf4654c0a | ||
|
|
ca1c18bbbb | ||
|
|
68136c64c0 | ||
|
|
9f3e0b8cd3 | ||
|
|
e46c6bcccf | ||
|
|
bfdaed9365 |
+19
-5
@@ -6,9 +6,20 @@
|
|||||||
**/build
|
**/build
|
||||||
web/build
|
web/build
|
||||||
|
|
||||||
# Flutter mobile client — built separately on developer machines / Flutter CI.
|
# The Android client — built by its own job, never from this context. The APK
|
||||||
# Including it in the Go build context wastes ~70 files and invalidates the
|
# reaches the image through client/, downloaded as a CI artifact, so nothing
|
||||||
# `COPY . .` layer cache on every Flutter-only change.
|
# here reads android/ sources.
|
||||||
|
#
|
||||||
|
# This block named `flutter_client/` until 2026-09-10 and lost its PATTERN when
|
||||||
|
# that tree was deleted, leaving a comment describing an exclusion that was no
|
||||||
|
# longer happening. android/ never took its place, so 4.1 MB of Gradle project
|
||||||
|
# has been entering the context and busting the `COPY . .` layer on every
|
||||||
|
# Android-only change.
|
||||||
|
android/
|
||||||
|
|
||||||
|
# Local `make build` output — an 18 MB binary the image never uses, since the
|
||||||
|
# builder stage compiles its own.
|
||||||
|
bin/
|
||||||
|
|
||||||
# Docs and IDE noise
|
# Docs and IDE noise
|
||||||
docs/
|
docs/
|
||||||
@@ -26,5 +37,8 @@ docs/
|
|||||||
!.env.example
|
!.env.example
|
||||||
|
|
||||||
# CI workflow files don't need to ship in the image.
|
# CI workflow files don't need to ship in the image.
|
||||||
.forgejo/
|
#
|
||||||
.github/
|
# This said `.forgejo/` and `.github/` — neither of which this repo has. Gitea
|
||||||
|
# Actions reads `.gitea/`, so the one directory that actually exists was the
|
||||||
|
# one not excluded, and every workflow edit invalidated the context.
|
||||||
|
.gitea/
|
||||||
|
|||||||
@@ -80,15 +80,12 @@ jobs:
|
|||||||
|
|
||||||
- name: Upload debug APK
|
- name: Upload debug APK
|
||||||
if: github.event_name == 'push' && github.ref == 'refs/heads/main'
|
if: github.event_name == 'push' && github.ref == 'refs/heads/main'
|
||||||
# Mirrored action, never actions/upload-artifact. @v4+ throws
|
# Stock action: it works on this forge since the runner moved to
|
||||||
# GHESNotSupportedError client-side on the hostname (no server setting
|
# gitea/runner 3.x, which edits upload-artifact's client-side GHES refusal
|
||||||
# reaches that check), and @v3 is worse — it reports success while Gitea
|
# out of the action bundle (Scribe snippet #2271). Never @v3 — it reports
|
||||||
# serves artifacts back only through the v4 API, so the upload is stored
|
# success while Gitea serves artifacts back only through the v4 API, and
|
||||||
# and invisible to every retrieval path. @v3 is what left 72 unreachable
|
# it is what left 72 unreachable artifacts on this repo (Scribe 2270).
|
||||||
# artifacts on this repo. Pinned by SHA because the mirror auto-syncs;
|
uses: actions/upload-artifact@v7
|
||||||
# full URL because DEFAULT_ACTIONS_URL sends bare owner/repo to github.com.
|
|
||||||
# See Scribe issues 2255 / 2270.
|
|
||||||
uses: https://git.fabledsword.com/bvandeusen/upload-artifact@cb8afe72b42edc798abfb8fcb556cf660d894245
|
|
||||||
with:
|
with:
|
||||||
name: minstrel-android-debug-${{ github.sha }}
|
name: minstrel-android-debug-${{ github.sha }}
|
||||||
path: android/app/build/outputs/apk/debug/app-debug.apk
|
path: android/app/build/outputs/apk/debug/app-debug.apk
|
||||||
|
|||||||
+291
-93
@@ -2,15 +2,71 @@ name: release
|
|||||||
|
|
||||||
# Builds and pushes the minstrel container image to the Gitea registry.
|
# Builds and pushes the minstrel container image to the Gitea registry.
|
||||||
#
|
#
|
||||||
# push to main → :main and :latest (latest-release APK bundled)
|
# push to dev → :dev (freshly-built dev APK bundled)
|
||||||
# push tag vYYYY.MM.DD → :vYYYY.MM.DD and :latest (freshly-built APK bundled)
|
# push to main → :latest + :<sha> (latest-release APK bundled)
|
||||||
# workflow_dispatch → manual trigger (same rules based on the ref)
|
# push tag vYYYY.MM.DD.HHMM → :latest (fresh APK bundled)
|
||||||
|
# workflow_dispatch → manual trigger (same rules based on the ref)
|
||||||
#
|
#
|
||||||
# Release model: per-day CalVer tags (no trailing patch digit). The day's
|
# That is the whole tag map, and it is family rule 145 + 147 as written.
|
||||||
# tag is intentionally mutable — if a second release happens the same day,
|
#
|
||||||
# move the tag with `git push -f origin vYYYY.MM.DD` and the image tag of
|
# :<sha> on main is the ROLLBACK UNIT — every production commit addressable
|
||||||
# the same name gets overwritten. :latest is updated by every main push
|
# without a release ceremony. It is minted only on main, where rollback is
|
||||||
# AND every tag push, so it always reflects the newest blessed image.
|
# actually worth having: merges are gated (rule 2) so they number in the dozens
|
||||||
|
# per year, while on dev they would be one per push, forever, for a channel
|
||||||
|
# whose entire contract is that it moves.
|
||||||
|
#
|
||||||
|
# There are NO :<version> image tags. This repo published :vYYYY.MM.DD.HHMM
|
||||||
|
# until 2026-09-10 and it was the inverse of the rule on both counts — minting
|
||||||
|
# a version tag nobody pinned while the rollback unit the rule names did not
|
||||||
|
# exist here at all. Git and the build's own self-reported version answer
|
||||||
|
# "which build is this"; a third name for the same thing is upkeep for a model
|
||||||
|
# we do not run. Operator, 2026-09-10: "only things like the APK need that kind
|
||||||
|
# of versioning for their update process."
|
||||||
|
#
|
||||||
|
# There is no :main either. :latest tracks main's tip with no gate between them
|
||||||
|
# (rule 147), so a second name for the same image sends readers looking for a
|
||||||
|
# distinction that does not exist.
|
||||||
|
#
|
||||||
|
# The dev channel exists so testing a build does not require shipping one.
|
||||||
|
# Before it, the only way to get an APK onto a phone was to cut a release,
|
||||||
|
# which made `main` the staging area by default. `:dev` carries its own
|
||||||
|
# freshly-built APK, signed with the SAME key as release builds — a different
|
||||||
|
# key cannot install over the stable app, so anyone crossing channels would
|
||||||
|
# have to uninstall and lose their data.
|
||||||
|
#
|
||||||
|
# :dev is published ALONE, with no per-commit tag. A rolling channel is
|
||||||
|
# rolling by definition; a commit-addressable image for it would be a
|
||||||
|
# rollback target nobody ever pulls, kept forever. Recovery on dev is to fix
|
||||||
|
# forward.
|
||||||
|
#
|
||||||
|
# Note what this repo does NOT need: a cross-repo dispatch to refresh the
|
||||||
|
# channel when its bundled APK is rebuilt. That mechanism exists elsewhere in
|
||||||
|
# the family because the app and the server live in separate repos. Minstrel
|
||||||
|
# is a monorepo — one push builds the APK and the image in the same run from
|
||||||
|
# the same commit, so the channel cannot go stale against its own artifact.
|
||||||
|
# The requirement is satisfied structurally; copying the mechanism would add
|
||||||
|
# a moving part to fix a problem that does not exist here.
|
||||||
|
#
|
||||||
|
# Release model: the tag IS the artifact's version name with a `v` in front.
|
||||||
|
# `v2026.09.10.1432` and `2026.09.10.1432` are the same string, derived from
|
||||||
|
# the tagged commit's UTC timestamp — so there is no mismatch to reconcile
|
||||||
|
# between what the tag says and what the APK reports, and nothing to look up
|
||||||
|
# when minting one.
|
||||||
|
#
|
||||||
|
# TAGS ARE IMMUTABLE. Never move, retarget or delete a published tag. A
|
||||||
|
# same-day second release is not a collision — HHMM makes every tag unique
|
||||||
|
# by construction, so the answer is simply another tag.
|
||||||
|
#
|
||||||
|
# This block used to say the opposite: that the per-day tag was
|
||||||
|
# "intentionally mutable" and that a same-day re-cut should
|
||||||
|
# `git push -f origin vYYYY.MM.DD`. That instruction is what the family
|
||||||
|
# rulebook now forbids outright, and it has incidents behind it — moving a
|
||||||
|
# same-day tag forward once took a published release down with it. Anyone
|
||||||
|
# installing from a tag is holding something the tag no longer points at,
|
||||||
|
# which is a worse failure than an extra row in the tag list.
|
||||||
|
#
|
||||||
|
# :latest is updated by every main push AND every tag push, so it always
|
||||||
|
# reflects the newest blessed image.
|
||||||
#
|
#
|
||||||
# APK pipeline: on tag pushes the android-release job builds + signs the
|
# APK pipeline: on tag pushes the android-release job builds + signs the
|
||||||
# Android APK and uploads it as a workflow artifact. The image-release
|
# Android APK and uploads it as a workflow artifact. The image-release
|
||||||
@@ -24,33 +80,37 @@ name: release
|
|||||||
# :latest (not just tags), a main build with no APK would silently strip
|
# :latest (not just tags), a main build with no APK would silently strip
|
||||||
# the in-app update channel off :latest until the next release. So on
|
# the in-app update channel off :latest until the next release. So on
|
||||||
# non-tag builds image-release pulls the MOST RECENT release's signed APK
|
# non-tag builds image-release pulls the MOST RECENT release's signed APK
|
||||||
# and reconstructs its exact versionName (tag + commit-count, the same
|
# AND the version sidecar published beside it — the recorded values, not
|
||||||
# formula android-release bakes in) for the version sidecar — no rebuild,
|
# recomputed ones — so no rebuild is needed, just a rebundle. Tag builds
|
||||||
# just rebundle. Tag builds keep bundling their own freshly-built APK.
|
# keep bundling their own freshly-built APK.
|
||||||
#
|
#
|
||||||
# Android testing (lint + detekt + unit tests, debug APK upload on main)
|
# Android testing (lint + detekt + unit tests, debug APK upload on main)
|
||||||
# lives in android.yml and runs independently on every push.
|
# lives in android.yml and runs independently on every push.
|
||||||
|
|
||||||
on:
|
on:
|
||||||
push:
|
push:
|
||||||
branches: [main]
|
branches: [main, dev]
|
||||||
tags: ['v*']
|
tags: ['v*']
|
||||||
paths-ignore:
|
paths-ignore:
|
||||||
- 'docs/**'
|
- 'docs/**'
|
||||||
- '**/*.md'
|
- '**/*.md'
|
||||||
workflow_dispatch:
|
workflow_dispatch:
|
||||||
|
|
||||||
# Force-moving the per-day tag (or rapidly re-pushing to main) should
|
# A rapid re-push to main should supersede the in-flight build — the
|
||||||
# supersede the in-flight build — the operator explicitly wants the
|
# operator explicitly wants the later commit to win. Tags no longer enter
|
||||||
# later commit to win.
|
# into this: they are immutable and unique, so no tag build can ever be
|
||||||
|
# superseded by another run on the same ref.
|
||||||
concurrency:
|
concurrency:
|
||||||
group: ${{ github.workflow }}-${{ github.ref }}
|
group: ${{ github.workflow }}-${{ github.ref }}
|
||||||
cancel-in-progress: true
|
cancel-in-progress: true
|
||||||
|
|
||||||
jobs:
|
jobs:
|
||||||
android-release:
|
android-release:
|
||||||
name: Build signed APK (tag releases only)
|
name: Build signed APK (releases and dev)
|
||||||
if: startsWith(github.ref, 'refs/tags/v')
|
# Also builds on `dev`, which is what makes a test channel possible at
|
||||||
|
# all. Without it the only way to get a build onto a phone was to cut a
|
||||||
|
# release, which quietly turns `main` into the staging area.
|
||||||
|
if: startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/dev'
|
||||||
runs-on: flutter-ci
|
runs-on: flutter-ci
|
||||||
container:
|
container:
|
||||||
image: git.fabledsword.com/bvandeusen/ci-android:36
|
image: git.fabledsword.com/bvandeusen/ci-android:36
|
||||||
@@ -75,14 +135,18 @@ jobs:
|
|||||||
outputs:
|
outputs:
|
||||||
version_name: ${{ steps.ver.outputs.name }}
|
version_name: ${{ steps.ver.outputs.name }}
|
||||||
version_code: ${{ steps.ver.outputs.code }}
|
version_code: ${{ steps.ver.outputs.code }}
|
||||||
|
channel: ${{ steps.ver.outputs.channel }}
|
||||||
|
|
||||||
steps:
|
steps:
|
||||||
- name: Checkout
|
- name: Checkout
|
||||||
uses: actions/checkout@v4
|
uses: actions/checkout@v4
|
||||||
with:
|
with:
|
||||||
# fetch-depth: 0 retrieves full history; default shallow clone
|
# Full history. The version name now reads only the tip commit's
|
||||||
# would return 1 for `git rev-list --count HEAD`, breaking the
|
# timestamp, so a shallow clone would technically serve — but this
|
||||||
# iteration suffix.
|
# job derives a value that ships to devices, and a shallow checkout
|
||||||
|
# changes what git-derived values resolve to WITHOUT failing. The
|
||||||
|
# whole failure class here is a green build carrying a wrong
|
||||||
|
# version, so the cheap guarantee is worth keeping.
|
||||||
fetch-depth: 0
|
fetch-depth: 0
|
||||||
|
|
||||||
- name: Compute release version
|
- name: Compute release version
|
||||||
@@ -91,12 +155,23 @@ jobs:
|
|||||||
working-directory: ${{ github.workspace }}
|
working-directory: ${{ github.workspace }}
|
||||||
run: |
|
run: |
|
||||||
set -euo pipefail
|
set -euo pipefail
|
||||||
TAG="${GITHUB_REF#refs/tags/v}"
|
# The derivation lives in ci/version.sh, not here, so it can be
|
||||||
COMMIT_COUNT=$(git rev-list --count HEAD)
|
# executed by a test on every push. Anything inline in this file is
|
||||||
VERSION_NAME="${TAG}.${COMMIT_COUNT}"
|
# unverifiable until a release is already running.
|
||||||
echo "name=${VERSION_NAME}" >> "$GITHUB_OUTPUT"
|
out="$(ci/version.sh HEAD)"
|
||||||
echo "code=${COMMIT_COUNT}" >> "$GITHUB_OUTPUT"
|
printf '%s\n' "${out}" >> "$GITHUB_OUTPUT"
|
||||||
echo "::notice::APK version: ${VERSION_NAME} (code=${COMMIT_COUNT})"
|
|
||||||
|
# The channel is a property of the LANE, not of the commit, which is
|
||||||
|
# why it is derived here rather than in version.sh. Same commit built
|
||||||
|
# on dev and on main reports the same NAME and differs only here —
|
||||||
|
# that is the whole point of separating the two values.
|
||||||
|
if [ "${GITHUB_REF}" = "refs/heads/dev" ]; then
|
||||||
|
channel=dev
|
||||||
|
else
|
||||||
|
channel=stable
|
||||||
|
fi
|
||||||
|
echo "channel=${channel}" >> "$GITHUB_OUTPUT"
|
||||||
|
echo "::notice::APK $(printf '%s' "${out}" | tr '\n' ' ') channel=${channel}"
|
||||||
|
|
||||||
# Checked BEFORE the expensive work, not after it. "Attach APK to gitea
|
# Checked BEFORE the expensive work, not after it. "Attach APK to gitea
|
||||||
# Release" below resolves the release by tag and fails if it is absent —
|
# Release" below resolves the release by tag and fails if it is absent —
|
||||||
@@ -108,6 +183,7 @@ jobs:
|
|||||||
# the release together, so this passes). A bare `git push origin vX` is the
|
# the release together, so this passes). A bare `git push origin vX` is the
|
||||||
# case this catches.
|
# case this catches.
|
||||||
- name: Release must exist for this tag
|
- name: Release must exist for this tag
|
||||||
|
if: startsWith(github.ref, 'refs/tags/v')
|
||||||
shell: bash
|
shell: bash
|
||||||
working-directory: ${{ github.workspace }}
|
working-directory: ${{ github.workspace }}
|
||||||
env:
|
env:
|
||||||
@@ -156,13 +232,12 @@ jobs:
|
|||||||
-PMINSTREL_VERSION_CODE=${{ steps.ver.outputs.code }}
|
-PMINSTREL_VERSION_CODE=${{ steps.ver.outputs.code }}
|
||||||
|
|
||||||
- name: Upload APK as workflow artifact
|
- name: Upload APK as workflow artifact
|
||||||
# Mirrored action, never actions/upload-artifact — @v4+ refuses on the
|
# Stock action (snippet #2271) — never @v3, which uploads something Gitea
|
||||||
# hostname, @v3 uploads something Gitea will never serve back. This is
|
# will never serve back. This is the producing half of a pair:
|
||||||
# the producing half of a pair: image-release downloads `minstrel-apk`
|
# image-release downloads `minstrel-apk` below. Any upload v4+ pairs with
|
||||||
# below with the matching download-artifact mirror. Both must stay on
|
# any download v4+ on this forge (every combination tested 2026-09-10,
|
||||||
# the v4 protocol — mixing a v3 upload with a v4 download (or the
|
# Scribe spike #3843), so the two pins need not move together.
|
||||||
# reverse) yields an empty listing, not an error. See Scribe 2255 / 2270.
|
uses: actions/upload-artifact@v7
|
||||||
uses: https://git.fabledsword.com/bvandeusen/upload-artifact@cb8afe72b42edc798abfb8fcb556cf660d894245
|
|
||||||
with:
|
with:
|
||||||
name: minstrel-apk
|
name: minstrel-apk
|
||||||
path: android/app/build/outputs/apk/release/app-release.apk
|
path: android/app/build/outputs/apk/release/app-release.apk
|
||||||
@@ -171,9 +246,15 @@ jobs:
|
|||||||
if-no-files-found: error
|
if-no-files-found: error
|
||||||
|
|
||||||
- name: Attach APK to gitea Release
|
- name: Attach APK to gitea Release
|
||||||
|
# Tag releases only. A dev build has no Release to hang assets on and
|
||||||
|
# does not need one — the :dev image bundles the APK, and the server
|
||||||
|
# serves it from /api/client/apk like any other.
|
||||||
|
if: startsWith(github.ref, 'refs/tags/v')
|
||||||
shell: bash
|
shell: bash
|
||||||
env:
|
env:
|
||||||
CI_TOKEN: ${{ secrets.CI_TOKEN }}
|
CI_TOKEN: ${{ secrets.CI_TOKEN }}
|
||||||
|
VERSION_NAME: ${{ steps.ver.outputs.name }}
|
||||||
|
VERSION_CODE: ${{ steps.ver.outputs.code }}
|
||||||
run: |
|
run: |
|
||||||
set -euxo pipefail
|
set -euxo pipefail
|
||||||
TAG="${GITHUB_REF#refs/tags/}"
|
TAG="${GITHUB_REF#refs/tags/}"
|
||||||
@@ -181,6 +262,20 @@ jobs:
|
|||||||
APK_PATH="app/build/outputs/apk/release/app-release.apk"
|
APK_PATH="app/build/outputs/apk/release/app-release.apk"
|
||||||
ls -lh "${APK_PATH}"
|
ls -lh "${APK_PATH}"
|
||||||
|
|
||||||
|
# Publish the version sidecar as a release asset next to the APK.
|
||||||
|
#
|
||||||
|
# This is what lets a later :latest build stop RECONSTRUCTING the
|
||||||
|
# bundled APK's version and simply read what was recorded. The
|
||||||
|
# ordering key in particular cannot be re-derived after the fact —
|
||||||
|
# it is build-time minutes, so once this job ends the value exists
|
||||||
|
# nowhere else. Reconstruction could only ever recover the name,
|
||||||
|
# and only by duplicating a formula that then has to be kept in
|
||||||
|
# step across two files.
|
||||||
|
SIDECAR_PATH="/tmp/minstrel.apk.version"
|
||||||
|
printf '{"name":"%s","code":%s,"channel":"stable"}\n' \
|
||||||
|
"${VERSION_NAME}" "${VERSION_CODE}" > "${SIDECAR_PATH}"
|
||||||
|
cat "${SIDECAR_PATH}"
|
||||||
|
|
||||||
RELEASE_JSON="$(curl -fsSL \
|
RELEASE_JSON="$(curl -fsSL \
|
||||||
-H "Authorization: token ${CI_TOKEN}" \
|
-H "Authorization: token ${CI_TOKEN}" \
|
||||||
"https://git.fabledsword.com/api/v1/repos/${REPO}/releases/tags/${TAG}")"
|
"https://git.fabledsword.com/api/v1/repos/${REPO}/releases/tags/${TAG}")"
|
||||||
@@ -202,6 +297,20 @@ jobs:
|
|||||||
exit 1
|
exit 1
|
||||||
fi
|
fi
|
||||||
|
|
||||||
|
# Same treatment for the sidecar. Named `.apk.version` so the
|
||||||
|
# downloader's `\.apk$` match cannot pick it up by mistake.
|
||||||
|
SIDECAR_HTTP=$(curl -sS -L -o /tmp/upload-sidecar.out -w '%{http_code}' \
|
||||||
|
-H "Authorization: token ${CI_TOKEN}" \
|
||||||
|
-F "attachment=@${SIDECAR_PATH}" \
|
||||||
|
"https://git.fabledsword.com/api/v1/repos/${REPO}/releases/${RELEASE_ID}/assets?name=minstrel-${TAG}.apk.version")
|
||||||
|
echo "sidecar_upload_http=${SIDECAR_HTTP}"
|
||||||
|
cat /tmp/upload-sidecar.out || true
|
||||||
|
echo
|
||||||
|
if [ "${SIDECAR_HTTP}" -lt 200 ] || [ "${SIDECAR_HTTP}" -ge 300 ]; then
|
||||||
|
echo "::error::version sidecar upload returned HTTP ${SIDECAR_HTTP}"
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
|
||||||
image-release:
|
image-release:
|
||||||
name: Build + push container image
|
name: Build + push container image
|
||||||
# `needs:` waits for android-release. For tag pushes android-release
|
# `needs:` waits for android-release. For tag pushes android-release
|
||||||
@@ -222,11 +331,16 @@ jobs:
|
|||||||
- name: Checkout
|
- name: Checkout
|
||||||
uses: actions/checkout@v4
|
uses: actions/checkout@v4
|
||||||
with:
|
with:
|
||||||
# Full history + tags so non-tag :latest builds can resolve the
|
# Full history, and rule 149 names this specifically: any job that
|
||||||
# latest release tag's commit count and reconstruct the bundled
|
# DERIVES the version name needs it, because a shallow clone changes
|
||||||
# APK's exact versionName (see "Bundle latest release APK" below).
|
# what git-derived values resolve to WITHOUT failing — a too-low
|
||||||
|
# value, silently, with every lane green.
|
||||||
|
#
|
||||||
|
# This job was depth-1 while it took the version from GITHUB_REF. It
|
||||||
|
# now runs ci/version.sh itself, because with :<version> image tags
|
||||||
|
# gone the server's self-reported version is the only thing that says
|
||||||
|
# which build an image is.
|
||||||
fetch-depth: 0
|
fetch-depth: 0
|
||||||
fetch-tags: true
|
|
||||||
|
|
||||||
- name: Detect buildable project
|
- name: Detect buildable project
|
||||||
id: guard
|
id: guard
|
||||||
@@ -244,21 +358,68 @@ jobs:
|
|||||||
if: steps.guard.outputs.ready == 'true'
|
if: steps.guard.outputs.ready == 'true'
|
||||||
shell: bash
|
shell: bash
|
||||||
run: |
|
run: |
|
||||||
if [[ "${GITHUB_REF}" == refs/tags/v* ]]; then
|
set -euo pipefail
|
||||||
VERSION="${GITHUB_REF#refs/tags/}"
|
|
||||||
echo "args=-t ${IMAGE}:${VERSION} -t ${IMAGE}:latest" >> "$GITHUB_OUTPUT"
|
# THE VERSION, and it is derived the same way on every ref — the
|
||||||
echo "version=${VERSION}" >> "$GITHUB_OUTPUT"
|
# branch decides the CHANNEL, never the version (family rule 149).
|
||||||
echo "::notice::Release build: ${VERSION} + latest"
|
#
|
||||||
else
|
# This used to be three different things: the literal string "main"
|
||||||
# Main is the protected, post-PR-merge branch. Treat it as the
|
# on main, "dev" on dev, and the tag name on a tag. None of them
|
||||||
# rolling stable channel — every main push moves :latest.
|
# ordered, and the first two were the same string forever — two dev
|
||||||
# Pinned consumers can target :vYYYY.MM.DD; everyone else
|
# images eight weeks apart were indistinguishable in the UI. That
|
||||||
# gets the newest main.
|
# mattered little while :vYYYY.MM.DD.HHMM existed to identify a
|
||||||
echo "args=-t ${IMAGE}:main -t ${IMAGE}:latest" >> "$GITHUB_OUTPUT"
|
# build; with version image tags gone, this IS how an operator tells
|
||||||
echo "version=main" >> "$GITHUB_OUTPUT"
|
# which build a container is running.
|
||||||
echo "::notice::Main-branch build: :main + :latest"
|
#
|
||||||
|
# `sed -n s///p` rather than `grep`: it exits 0 when nothing matches,
|
||||||
|
# so the empty check below is actually reachable. A grep here would
|
||||||
|
# kill the step at the assignment under the runner's pipefail — the
|
||||||
|
# exact bug that took down the first main build after the version
|
||||||
|
# rework.
|
||||||
|
VERSION="$(ci/version.sh HEAD | sed -n 's/^name=//p')"
|
||||||
|
if [ -z "${VERSION}" ]; then
|
||||||
|
echo "::error::could not derive a build version from ci/version.sh"
|
||||||
|
exit 1
|
||||||
fi
|
fi
|
||||||
|
|
||||||
|
if [[ "${GITHUB_REF}" == refs/tags/v* ]]; then
|
||||||
|
# A release refreshes the CHANNEL and mints nothing else.
|
||||||
|
#
|
||||||
|
# The tag build exists to produce the signed APK and attach it to
|
||||||
|
# the release; the image it rebuilds is the SAME SOURCE as the main
|
||||||
|
# build minutes earlier, differing only in which APK is baked in.
|
||||||
|
# Rule 145 is explicit about that case: when the same source is
|
||||||
|
# rebuilt with different contents, publish the moving channel tag
|
||||||
|
# and never a commit-addressable one.
|
||||||
|
#
|
||||||
|
# :latest must move here rather than waiting for the next main
|
||||||
|
# push, or the channel would carry the PREVIOUS release's APK
|
||||||
|
# indefinitely — a channel that cannot refresh itself (rule 146).
|
||||||
|
CHANNEL=stable
|
||||||
|
echo "args=-t ${IMAGE}:latest" >> "$GITHUB_OUTPUT"
|
||||||
|
echo "::notice::Release build ${VERSION}: refreshing :latest around the new APK"
|
||||||
|
elif [[ "${GITHUB_REF}" == "refs/heads/dev" ]]; then
|
||||||
|
# The rolling test channel, and :dev ALONE — deliberately no
|
||||||
|
# per-commit tag. A rolling channel is rolling by definition, so a
|
||||||
|
# commit-addressable image here would be a rollback target nobody
|
||||||
|
# has ever pulled, accumulating in the registry forever. Recovery
|
||||||
|
# on dev is to fix forward.
|
||||||
|
CHANNEL=dev
|
||||||
|
echo "args=-t ${IMAGE}:dev" >> "$GITHUB_OUTPUT"
|
||||||
|
echo "::notice::Dev-branch build ${VERSION}: :dev"
|
||||||
|
else
|
||||||
|
# The production line: :latest tracks main's tip (rule 147) and
|
||||||
|
# :<sha> is the rollback unit (rule 145). Full 40-char SHA, matching
|
||||||
|
# the family's other repos, so a rollback target is addressable
|
||||||
|
# straight from the commit anyone is reading.
|
||||||
|
CHANNEL=stable
|
||||||
|
echo "args=-t ${IMAGE}:latest -t ${IMAGE}:${GITHUB_SHA}" >> "$GITHUB_OUTPUT"
|
||||||
|
echo "::notice::Main-branch build ${VERSION}: :latest + :${GITHUB_SHA}"
|
||||||
|
fi
|
||||||
|
|
||||||
|
echo "version=${VERSION}" >> "$GITHUB_OUTPUT"
|
||||||
|
echo "channel=${CHANNEL}" >> "$GITHUB_OUTPUT"
|
||||||
|
|
||||||
- name: Registry login
|
- name: Registry login
|
||||||
if: steps.guard.outputs.ready == 'true'
|
if: steps.guard.outputs.ready == 'true'
|
||||||
shell: bash
|
shell: bash
|
||||||
@@ -267,54 +428,57 @@ jobs:
|
|||||||
| docker login git.fabledsword.com -u "${{ github.actor }}" --password-stdin
|
| docker login git.fabledsword.com -u "${{ github.actor }}" --password-stdin
|
||||||
|
|
||||||
- name: Download signed APK artifact
|
- name: Download signed APK artifact
|
||||||
# Tag pushes only — android-release just produced this. Non-tag
|
# Tag and dev pushes — android-release just produced this. Only `main`
|
||||||
# builds take the "Bundle latest release APK" path below instead.
|
# takes the "Bundle latest release APK" path below, because it is the
|
||||||
if: steps.guard.outputs.ready == 'true' && startsWith(github.ref, 'refs/tags/v')
|
# one ref that moves a channel without building an APK of its own.
|
||||||
# Consuming half of the pair — never actions/download-artifact. Same fork,
|
if: >-
|
||||||
# same reason: upstream's client-side GHES check rejects this hostname
|
steps.guard.outputs.ready == 'true' &&
|
||||||
# before it connects. bvandeusen/download-artifact mirrors
|
(startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/dev')
|
||||||
# code.forgejo.org/forgejo/download-artifact.
|
# Consuming half of the pair: stock download-artifact, which works here for
|
||||||
#
|
# the same reason as the upload (gitea/runner 3.x edits the GHES refusal
|
||||||
# SHA below is that fork's `v6` tag. Match on @actions/artifact, NOT on
|
# out of the bundle; snippet #2271). v8 runs on node24, which every
|
||||||
# the action's own version number — the two actions release on unrelated
|
# CI-runner image carries — the runner uses the image's own node.
|
||||||
# cadences, and download v5 would pair a ^2.3.2 client with this file's
|
uses: actions/download-artifact@v8
|
||||||
# ^4.0.0 uploader. v6 is the tag whose bundled library major (^4.0.0) is
|
|
||||||
# the same one proven against this instance by the upload side.
|
|
||||||
# Deliberately NOT v7: it moves to node24 and upstream requires runner
|
|
||||||
# >= 2.327.1 for it, which act_runner does not claim to satisfy.
|
|
||||||
# Pinned, not tagged — the mirror auto-syncs every 8h.
|
|
||||||
uses: https://git.fabledsword.com/bvandeusen/download-artifact@8d4e9521a5f7e5f8b6351f341f719f9f45a92a3a
|
|
||||||
with:
|
with:
|
||||||
name: minstrel-apk
|
name: minstrel-apk
|
||||||
path: client/
|
path: client/
|
||||||
|
|
||||||
- name: Stage bundled APK + version sidecar
|
- name: Stage bundled APK + version sidecar
|
||||||
if: steps.guard.outputs.ready == 'true' && startsWith(github.ref, 'refs/tags/v')
|
if: >-
|
||||||
|
steps.guard.outputs.ready == 'true' &&
|
||||||
|
(startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/dev')
|
||||||
shell: bash
|
shell: bash
|
||||||
env:
|
env:
|
||||||
# Pulled from android-release.outputs.version_name so the
|
# All three pulled from android-release's outputs so the sidecar the
|
||||||
# sidecar string the server hands clients matches the
|
# server hands clients matches exactly what is baked into the APK
|
||||||
# versionName baked into the APK they're comparing against.
|
# they are comparing against.
|
||||||
APK_VERSION_NAME: ${{ needs.android-release.outputs.version_name }}
|
APK_VERSION_NAME: ${{ needs.android-release.outputs.version_name }}
|
||||||
|
APK_VERSION_CODE: ${{ needs.android-release.outputs.version_code }}
|
||||||
|
APK_CHANNEL: ${{ needs.android-release.outputs.channel }}
|
||||||
run: |
|
run: |
|
||||||
set -euxo pipefail
|
set -euxo pipefail
|
||||||
# The artifact lands as `app-release.apk` (the original Gradle
|
# The artifact lands as `app-release.apk` (the original Gradle
|
||||||
# output name). The Dockerfile COPYs client/* into /app/client/
|
# output name). The Dockerfile COPYs client/* into /app/client/
|
||||||
# and the server reads minstrel.apk + minstrel.apk.version.
|
# and the server reads minstrel.apk + minstrel.apk.version.
|
||||||
mv client/app-release.apk client/minstrel.apk
|
mv client/app-release.apk client/minstrel.apk
|
||||||
echo "${APK_VERSION_NAME}" > client/minstrel.apk.version
|
printf '{"name":"%s","code":%s,"channel":"%s"}\n' \
|
||||||
|
"${APK_VERSION_NAME}" "${APK_VERSION_CODE}" "${APK_CHANNEL}" \
|
||||||
|
> client/minstrel.apk.version
|
||||||
|
cat client/minstrel.apk.version
|
||||||
ls -lh client/
|
ls -lh client/
|
||||||
|
|
||||||
- name: Bundle latest release APK (non-tag :latest builds)
|
- name: Bundle latest release APK (non-tag :latest builds)
|
||||||
# Main pushes don't build an APK, but they DO move :latest — so
|
# Main pushes don't build an APK, but they DO move :latest — so
|
||||||
# without this the in-app update channel would vanish from :latest
|
# without this the in-app update channel would vanish from :latest
|
||||||
# until the next tag. Pull the most-recent release's signed APK and
|
# until the next tag. Pull the most-recent release's signed APK and
|
||||||
# reconstruct its exact versionName (${TAG#v}.$(git rev-list --count
|
# the sidecar published beside it, so what the server reports is what
|
||||||
# TAG) — identical to android-release's formula) so the version
|
# that build actually recorded rather than something re-derived here.
|
||||||
# sidecar the server hands clients matches the installed build.
|
|
||||||
# Degrades to an empty client/ (404 update channel) — never a wrong
|
# Degrades to an empty client/ (404 update channel) — never a wrong
|
||||||
# version — if no release / APK asset / tag-count can be resolved.
|
# version — if no release or APK asset can be resolved. That
|
||||||
if: steps.guard.outputs.ready == 'true' && !startsWith(github.ref, 'refs/tags/v')
|
# degradation only actually works because the greps below carry
|
||||||
|
# `|| true`; under the runner's default pipefail a non-matching grep
|
||||||
|
# kills the step instead of falling through to the empty-case branch.
|
||||||
|
if: steps.guard.outputs.ready == 'true' && github.ref == 'refs/heads/main'
|
||||||
shell: bash
|
shell: bash
|
||||||
env:
|
env:
|
||||||
CI_TOKEN: ${{ secrets.CI_TOKEN }}
|
CI_TOKEN: ${{ secrets.CI_TOKEN }}
|
||||||
@@ -326,19 +490,40 @@ jobs:
|
|||||||
if [ -z "${REL_JSON}" ]; then
|
if [ -z "${REL_JSON}" ]; then
|
||||||
echo "::notice::no published release — image ships without bundled APK"; exit 0
|
echo "::notice::no published release — image ships without bundled APK"; exit 0
|
||||||
fi
|
fi
|
||||||
TAG="$(printf '%s' "${REL_JSON}" | grep -oP '"tag_name":\s*"\K[^"]+' | head -1)"
|
# `|| true` on every one of these, and it is load-bearing rather
|
||||||
APK_URL="$(printf '%s' "${REL_JSON}" | grep -oP '"browser_download_url":\s*"\K[^"]+' | grep -E '\.apk$' | head -1)"
|
# than defensive habit. The runner already invokes this shell as
|
||||||
|
# `bash -e -o pipefail`, so a pipeline whose grep matches NOTHING
|
||||||
|
# exits non-zero even though `head` succeeded — and the step dies at
|
||||||
|
# the assignment, before ever reaching the `if` written to handle the
|
||||||
|
# empty case. Every "degrades gracefully" branch below is unreachable
|
||||||
|
# without this.
|
||||||
|
TAG="$(printf '%s' "${REL_JSON}" | grep -oP '"tag_name":\s*"\K[^"]+' | head -1)" || true
|
||||||
|
APK_URL="$(printf '%s' "${REL_JSON}" | grep -oP '"browser_download_url":\s*"\K[^"]+' | grep -E '\.apk$' | head -1)" || true
|
||||||
if [ -z "${TAG}" ] || [ -z "${APK_URL}" ]; then
|
if [ -z "${TAG}" ] || [ -z "${APK_URL}" ]; then
|
||||||
echo "::notice::latest release '${TAG:-?}' has no APK asset — image ships without bundled APK"; exit 0
|
echo "::notice::latest release '${TAG:-?}' has no APK asset — image ships without bundled APK"; exit 0
|
||||||
fi
|
fi
|
||||||
COUNT="$(git rev-list --count "${TAG}" 2>/dev/null || true)"
|
|
||||||
if [ -z "${COUNT}" ]; then
|
|
||||||
echo "::notice::could not resolve commit count for ${TAG} (tag not fetched?) — skipping APK bundle"; exit 0
|
|
||||||
fi
|
|
||||||
VERSION_NAME="${TAG#v}.${COUNT}"
|
|
||||||
curl -fsSL -H "Authorization: token ${CI_TOKEN}" -o client/minstrel.apk "${APK_URL}"
|
curl -fsSL -H "Authorization: token ${CI_TOKEN}" -o client/minstrel.apk "${APK_URL}"
|
||||||
echo "${VERSION_NAME}" > client/minstrel.apk.version
|
|
||||||
echo "::notice::bundled release APK ${TAG} as version ${VERSION_NAME}"
|
# Take the version the release RECORDED rather than recomputing it.
|
||||||
|
# This used to re-derive the name from the tagged commit, which meant
|
||||||
|
# the formula lived in two files that had to be kept in step, and it
|
||||||
|
# could only ever recover the name — the ordering key is build-time
|
||||||
|
# minutes and does not exist anywhere after that build ends.
|
||||||
|
SIDECAR_URL="$(printf '%s' "${REL_JSON}" | grep -oP '"browser_download_url":\s*"\K[^"]+' | grep -E '\.apk\.version$' | head -1)" || true
|
||||||
|
if [ -n "${SIDECAR_URL}" ]; then
|
||||||
|
curl -fsSL -H "Authorization: token ${CI_TOKEN}" -o client/minstrel.apk.version "${SIDECAR_URL}"
|
||||||
|
cat client/minstrel.apk.version
|
||||||
|
else
|
||||||
|
# Releases published before sidecars were attached. Their name is
|
||||||
|
# still recoverable from the tag, but their ordering key genuinely
|
||||||
|
# is not — so it is reported ABSENT rather than guessed. A wrong
|
||||||
|
# key is an install the platform refuses; an absent one just tells
|
||||||
|
# the client to fall back to comparing names, which is exactly
|
||||||
|
# what those builds already do.
|
||||||
|
echo "::notice::release ${TAG} predates the version sidecar — bundling with name only, no ordering key"
|
||||||
|
printf '{"name":"%s","code":null,"channel":"stable"}\n' "${TAG#v}" > client/minstrel.apk.version
|
||||||
|
fi
|
||||||
|
echo "::notice::bundled release APK from ${TAG}"
|
||||||
ls -lh client/
|
ls -lh client/
|
||||||
|
|
||||||
- name: Build and push
|
- name: Build and push
|
||||||
@@ -346,6 +531,7 @@ jobs:
|
|||||||
run: |
|
run: |
|
||||||
docker buildx build \
|
docker buildx build \
|
||||||
--build-arg MINSTREL_VERSION="${{ steps.tags.outputs.version }}" \
|
--build-arg MINSTREL_VERSION="${{ steps.tags.outputs.version }}" \
|
||||||
|
--build-arg MINSTREL_CHANNEL="${{ steps.tags.outputs.channel }}" \
|
||||||
--push ${{ steps.tags.outputs.args }} .
|
--push ${{ steps.tags.outputs.args }} .
|
||||||
|
|
||||||
# Verifies a tag release actually ended up complete, and names the specific
|
# Verifies a tag release actually ended up complete, and names the specific
|
||||||
@@ -356,8 +542,8 @@ jobs:
|
|||||||
# `failure` with none executed and image-release showed `skipped`. The run was
|
# `failure` with none executed and image-release showed `skipped`. The run was
|
||||||
# red, but the *release page rendered fine*, and `main`'s own push build had
|
# red, but the *release page rendered fine*, and `main`'s own push build had
|
||||||
# already moved `:latest`, so the code was deployable and nothing looked
|
# already moved `:latest`, so the code was deployable and nothing looked
|
||||||
# obviously wrong. The release was simply missing its APK and its immutable
|
# obviously wrong. The release was simply missing its APK and its image,
|
||||||
# `:vYYYY.MM.DD` image, which is easy to skim past.
|
# which is easy to skim past.
|
||||||
#
|
#
|
||||||
# This job cannot prevent that (the cause was a runner failing to launch, not
|
# This job cannot prevent that (the cause was a runner failing to launch, not
|
||||||
# anything in this file). What it does is turn an incomplete release into an
|
# anything in this file). What it does is turn an incomplete release into an
|
||||||
@@ -408,18 +594,30 @@ jobs:
|
|||||||
# missing when v2026.08.07 had to be re-cut. `always()` on this job means
|
# missing when v2026.08.07 had to be re-cut. `always()` on this job means
|
||||||
# it runs even when image-release failed, so without this the guard would
|
# it runs even when image-release failed, so without this the guard would
|
||||||
# cheerfully verify an incomplete release.
|
# cheerfully verify an incomplete release.
|
||||||
- name: Immutable image tag must exist
|
#
|
||||||
|
# This asserted `:${TAG}` — the :vYYYY.MM.DD.HHMM image — until
|
||||||
|
# 2026-09-10. Version image tags are no longer published (rule 145), so
|
||||||
|
# that assertion would now fail every release for a tag nothing mints.
|
||||||
|
# The rollback target it was really protecting is the :<sha> image, which
|
||||||
|
# main's own build published for this same commit before the tag was cut.
|
||||||
|
#
|
||||||
|
# Checking it here earns its keep twice over: it still catches an image
|
||||||
|
# push that silently did not happen, and it additionally proves the
|
||||||
|
# ORDERING — a tag cut on a commit whose main build never completed has
|
||||||
|
# no rollback target, and that is worth failing on rather than
|
||||||
|
# discovering during an incident.
|
||||||
|
- name: Rollback image must exist for the tagged commit
|
||||||
shell: bash
|
shell: bash
|
||||||
run: |
|
run: |
|
||||||
set -euo pipefail
|
set -euo pipefail
|
||||||
TAG="${GITHUB_REF#refs/tags/}"
|
|
||||||
IMAGE="git.fabledsword.com/bvandeusen/minstrel"
|
IMAGE="git.fabledsword.com/bvandeusen/minstrel"
|
||||||
|
|
||||||
echo "${{ secrets.CI_TOKEN }}" \
|
echo "${{ secrets.CI_TOKEN }}" \
|
||||||
| docker login git.fabledsword.com -u "${{ github.actor }}" --password-stdin
|
| docker login git.fabledsword.com -u "${{ github.actor }}" --password-stdin
|
||||||
|
|
||||||
if ! docker manifest inspect "${IMAGE}:${TAG}" > /dev/null 2>&1; then
|
if ! docker manifest inspect "${IMAGE}:${GITHUB_SHA}" > /dev/null 2>&1; then
|
||||||
echo "::error::image ${IMAGE}:${TAG} was never pushed — the release tag has no immutable image, so there is nothing to pin or roll back to. Re-run this workflow run."
|
echo "::error::image ${IMAGE}:${GITHUB_SHA} does not exist — this commit has no rollback target."
|
||||||
|
echo "::error::That image is published by the MAIN build of this commit, not by the tag build. If main's build never ran or failed, fix that first; a release whose commit cannot be rolled back to is the thing this check exists to refuse."
|
||||||
exit 1
|
exit 1
|
||||||
fi
|
fi
|
||||||
echo "::notice::image verified: ${IMAGE}:${TAG}"
|
echo "::notice::rollback target verified: ${IMAGE}:${GITHUB_SHA}"
|
||||||
|
|||||||
@@ -32,6 +32,12 @@ on:
|
|||||||
- 'cmd/**'
|
- 'cmd/**'
|
||||||
- '.golangci.yml'
|
- '.golangci.yml'
|
||||||
- '.gitea/workflows/test-go.yml'
|
- '.gitea/workflows/test-go.yml'
|
||||||
|
# The release lane's own trigger is `main` + tags, so nothing it
|
||||||
|
# contains is exercised until a release is already running. These two
|
||||||
|
# entries are what let internal/server/release_version_test.go guard
|
||||||
|
# the version derivation on ordinary dev pushes instead.
|
||||||
|
- 'ci/**'
|
||||||
|
- '.gitea/workflows/release.yml'
|
||||||
|
|
||||||
# pull_request trigger intentionally omitted — see test-web.yml for
|
# pull_request trigger intentionally omitted — see test-web.yml for
|
||||||
# the rationale (single-author repo, push covers PR-merge equivalent).
|
# the rationale (single-author repo, push covers PR-merge equivalent).
|
||||||
|
|||||||
@@ -12,6 +12,11 @@
|
|||||||
# Test binary, built with `go test -c`
|
# Test binary, built with `go test -c`
|
||||||
*.test
|
*.test
|
||||||
|
|
||||||
|
# `make build` output. bin/minstrel was tracked until 2026-09-10 — an 18 MB
|
||||||
|
# binary committed by accident, last refreshed by a commit about web test
|
||||||
|
# mocks, and re-dirtied by every local build since.
|
||||||
|
bin/
|
||||||
|
|
||||||
# Bundled Android APK + version sidecar (#397). Populated by CI for
|
# Bundled Android APK + version sidecar (#397). Populated by CI for
|
||||||
# tag releases; never committed. README in client/ explains the flow.
|
# tag releases; never committed. README in client/ explains the flow.
|
||||||
client/minstrel.apk
|
client/minstrel.apk
|
||||||
|
|||||||
+13
-4
@@ -15,12 +15,21 @@ COPY . .
|
|||||||
# Overwrite the committed placeholder with the freshly-built SPA assets.
|
# Overwrite the committed placeholder with the freshly-built SPA assets.
|
||||||
COPY --from=web /web/build ./web/build
|
COPY --from=web /web/build ./web/build
|
||||||
ENV CGO_ENABLED=0
|
ENV CGO_ENABLED=0
|
||||||
# Version stamping: release.yml passes the git tag via MINSTREL_VERSION
|
# Version stamping. release.yml passes the DERIVED version name
|
||||||
# build-arg; local `docker build` falls back to "dev". Surfaced at
|
# (YYYY.MM.DD.HHMM) and the lane's channel; a local `docker build` falls back
|
||||||
# /healthz for operator-side image-version verification.
|
# to "dev"/"local". Both are surfaced at /healthz.
|
||||||
|
#
|
||||||
|
# These are two values on purpose (family rule 149): the same commit built on
|
||||||
|
# dev and on main reports the same NAME and differs only in CHANNEL. Folding
|
||||||
|
# the channel into the version string is what the rule forbids — the version
|
||||||
|
# used to BE the channel word here ("main"/"dev"), which meant two dev images
|
||||||
|
# eight weeks apart were indistinguishable.
|
||||||
ARG MINSTREL_VERSION=dev
|
ARG MINSTREL_VERSION=dev
|
||||||
|
ARG MINSTREL_CHANNEL=local
|
||||||
RUN go build -trimpath \
|
RUN go build -trimpath \
|
||||||
-ldflags="-s -w -X 'git.fabledsword.com/bvandeusen/minstrel/internal/server.ServerVersion=${MINSTREL_VERSION}'" \
|
-ldflags="-s -w \
|
||||||
|
-X 'git.fabledsword.com/bvandeusen/minstrel/internal/server.ServerVersion=${MINSTREL_VERSION}' \
|
||||||
|
-X 'git.fabledsword.com/bvandeusen/minstrel/internal/server.ServerChannel=${MINSTREL_CHANNEL}'" \
|
||||||
-o /out/minstrel ./cmd/minstrel
|
-o /out/minstrel ./cmd/minstrel
|
||||||
|
|
||||||
FROM debian:bookworm-slim
|
FROM debian:bookworm-slim
|
||||||
|
|||||||
@@ -112,11 +112,21 @@ Most operational keys have a `MINSTREL_<SECTION>_<FIELD>` env override. Recommen
|
|||||||
|
|
||||||
Image tags (`git.fabledsword.com/bvandeusen/minstrel:<tag>`):
|
Image tags (`git.fabledsword.com/bvandeusen/minstrel:<tag>`):
|
||||||
|
|
||||||
- `:latest` — the newest blessed image. Moves on every `main` push **and** every release. Recommended for most operators.
|
- `:latest` — production. Tracks `main`'s tip and moves on every `main` push and every release. What most operators should run.
|
||||||
- `:vYYYY.MM.DD` — immutable per-day release tags. Pin one of these for a deployment you don't want moving under you. (Per-day CalVer — no trailing patch digit; a same-day re-cut moves the tag forward.)
|
- `:<commit-sha>` — the rollback unit. Every `main` push publishes one, so any production commit is addressable without a release ceremony. Immutable: a given SHA tag is never re-pushed. Pin one if you need a deployment that cannot change under you, and use it to roll back.
|
||||||
- `:main` — the rolling post-merge tip. Same image as `:latest` at push time; choose it if you want to track `main` explicitly rather than the release line.
|
- `:dev` — the rolling test channel, rebuilt on every push to `dev` and carrying its own freshly-built Android APK. Run this to try something before it ships. It moves constantly, has no per-commit tag, and its only recovery path is forward — if a `:dev` image is broken, the fix is the next push, not a rollback.
|
||||||
|
|
||||||
Every `:latest` and every `:vYYYY.MM.DD` bundles the current signed Android APK, so the in-app update channel is always live. Database migrations run automatically at startup; rollbacks require restoring a Postgres dump.
|
That is the whole tag map. **There are no version-numbered image tags**, and no `:main`. Git and the build's own self-reported version answer "which build is this" — the Settings page shows it, and so does `/healthz`. Release *tags* in git are still `vYYYY.MM.DD.HHMM`; they name a changelog entry and the APK attached to it, not an image.
|
||||||
|
|
||||||
|
Rolling back to `:<commit-sha>` pins the **server code** at that commit — not the server-and-app pair. The Android APK is baked in at image build time, so a SHA image carries whichever app was current when that commit was built, which may be older than what `:latest` bundles now. If both halves matter, check what the image bundles rather than trusting the tag's name.
|
||||||
|
|
||||||
|
Every `:latest`, `:<commit-sha>` and `:dev` bundles a signed Android APK, so the in-app update channel is always live. All are signed with the same key, so a phone can move between the stable and dev channels without uninstalling — point it at a `:dev` server and the in-app updater offers that channel's build.
|
||||||
|
|
||||||
|
The app reports which channel it is on alongside its version, and decides whether an update is available using the build's ordering key rather than its displayed name — the same value Android installs by, so an offer it makes is one the platform will accept.
|
||||||
|
|
||||||
|
Database migrations run automatically at startup; rollbacks require restoring a Postgres dump.
|
||||||
|
|
||||||
|
Releases up to 2026-09-10 also published a `:vYYYY.MM.DD[.HHMM]` image tag. Those images still exist and still work — they are simply not extended.
|
||||||
|
|
||||||
## Specs
|
## Specs
|
||||||
|
|
||||||
@@ -150,7 +160,7 @@ Two concurrent dev processes:
|
|||||||
|
|
||||||
- Day-to-day work happens on `dev` (or feature branches merged into `dev`).
|
- Day-to-day work happens on `dev` (or feature branches merged into `dev`).
|
||||||
- `main` is **protected** — changes land via PR from `dev`.
|
- `main` is **protected** — changes land via PR from `dev`.
|
||||||
- Releases are cut by tagging `v*` off `main`; the release workflow builds and pushes the container image to the Gitea registry.
|
- Releases are cut by tagging `v*` off `main`; the release workflow builds the signed APK, attaches it to the release, and refreshes `:latest` around it.
|
||||||
|
|
||||||
Task and milestone tracking: Fable (`Minstrel` project, id 12).
|
Task and milestone tracking: Fable (`Minstrel` project, id 12).
|
||||||
|
|
||||||
|
|||||||
@@ -21,13 +21,24 @@ android {
|
|||||||
applicationId = "com.fabledsword.minstrel"
|
applicationId = "com.fabledsword.minstrel"
|
||||||
minSdk = 26
|
minSdk = 26
|
||||||
targetSdk = 36
|
targetSdk = 36
|
||||||
// versionName / versionCode are released-build values injected by
|
// versionName / versionCode are released-build values injected by CI.
|
||||||
// CI from the git tag + commit count. Local / debug builds fall
|
// Local / debug builds fall back to "dev" so the About card reads
|
||||||
// back to "dev" so the About card reads honestly. Releases ship
|
// honestly.
|
||||||
// versionName="YYYY.MM.DD.<commits>" (e.g. "2026.06.02.142") and
|
//
|
||||||
// versionCode=<commits>, which is monotonic forever and lets the
|
// versionName is "YYYY.MM.DD.HHMM" from the COMMIT's timestamp, so
|
||||||
// shared isVersionNewer comparator distinguish two same-day
|
// every lane building this source reports the same string and the
|
||||||
// re-cuts (the iteration suffix differs).
|
// channel is the only thing that differs between them.
|
||||||
|
//
|
||||||
|
// versionCode is minutes since 2020-01-01 at BUILD time. It is the
|
||||||
|
// value the platform decides installs by, so it must be monotonic by
|
||||||
|
// construction.
|
||||||
|
//
|
||||||
|
// This comment used to say versionCode was a commit count and that it
|
||||||
|
// was "monotonic forever". It was neither — a commit count runs ahead
|
||||||
|
// on `dev`, so a dev build outranked the `main` release meant to
|
||||||
|
// replace it and Android refused the install as a downgrade. Worth
|
||||||
|
// knowing the claim was here, stated as a reassurance, while the bug
|
||||||
|
// it denied was live.
|
||||||
val versionNameOverride =
|
val versionNameOverride =
|
||||||
(project.findProperty("MINSTREL_VERSION_NAME") as String?)?.takeIf { it.isNotBlank() }
|
(project.findProperty("MINSTREL_VERSION_NAME") as String?)?.takeIf { it.isNotBlank() }
|
||||||
val versionCodeOverride =
|
val versionCodeOverride =
|
||||||
|
|||||||
@@ -1,15 +1,26 @@
|
|||||||
package com.fabledsword.minstrel.models
|
package com.fabledsword.minstrel.models
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Wire shape returned by `GET /api/client/version`. Mirrors
|
* The server-bundled APK, as reported by `GET /api/client/version`.
|
||||||
* the Flutter client's `UpdateInfo`.
|
|
||||||
*
|
*
|
||||||
* `version` is the server-bundled APK version (may have a leading
|
* Three values that are deliberately kept apart:
|
||||||
* "v" from the git tag); `apkUrl` is server-relative (e.g.
|
*
|
||||||
* `/api/client/apk`); `sizeBytes` is the download size.
|
* - [version] is a LABEL for people — "YYYY.MM.DD.HHMM", derived from the
|
||||||
|
* build's commit, so two channels carrying the same code read the same.
|
||||||
|
* Display this; never decide on it when [code] is present.
|
||||||
|
* - [code] is the ORDERING KEY, and is the same value Android itself
|
||||||
|
* installs by. It answers "may this be installed over that?", which the
|
||||||
|
* name cannot. Null when the server predates the field.
|
||||||
|
* - [channel] is a SIBLING FIELD, never a suffix inside the name. Reported
|
||||||
|
* verbatim rather than validated, so an unexpected value is shown rather
|
||||||
|
* than dropped.
|
||||||
|
*
|
||||||
|
* [apkUrl] is server-relative (e.g. `/api/client/apk`).
|
||||||
*/
|
*/
|
||||||
data class UpdateInfo(
|
data class UpdateInfo(
|
||||||
val version: String,
|
val version: String,
|
||||||
|
val code: Long?,
|
||||||
|
val channel: String?,
|
||||||
val apkUrl: String,
|
val apkUrl: String,
|
||||||
val sizeBytes: Long,
|
val sizeBytes: Long,
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -4,12 +4,26 @@ import kotlinx.serialization.SerialName
|
|||||||
import kotlinx.serialization.Serializable
|
import kotlinx.serialization.Serializable
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Wire shape for `GET /api/client/version`. Defaults match Flutter:
|
* Wire shape for `GET /api/client/version`.
|
||||||
* apk_url falls back to `/api/client/apk` if the server omits it.
|
*
|
||||||
|
* `apkUrl` falls back to `/api/client/apk` if the server omits it.
|
||||||
|
*
|
||||||
|
* [code] MUST stay nullable, and this is not a style preference. The app's
|
||||||
|
* Json is configured with `coerceInputValues = true`, which replaces a JSON
|
||||||
|
* null with the declared default for a NON-nullable property — so writing
|
||||||
|
* `val code: Long = 0` would turn "this server reports no ordering key" into
|
||||||
|
* "this build's ordering key is 0", silently, with no error anywhere. A
|
||||||
|
* nullable type is what keeps absent distinguishable from zero, and the
|
||||||
|
* distinction is the whole reason the field exists.
|
||||||
|
*
|
||||||
|
* A server predating the ordering key sends neither [code] nor [channel];
|
||||||
|
* both arrive null and the caller falls back to comparing names.
|
||||||
*/
|
*/
|
||||||
@Serializable
|
@Serializable
|
||||||
data class UpdateInfoWire(
|
data class UpdateInfoWire(
|
||||||
val version: String = "",
|
val version: String = "",
|
||||||
|
val code: Long? = null,
|
||||||
|
val channel: String? = null,
|
||||||
@SerialName("apk_url") val apkUrl: String = "/api/client/apk",
|
@SerialName("apk_url") val apkUrl: String = "/api/client/apk",
|
||||||
@SerialName("size_bytes") val sizeBytes: Long = 0,
|
@SerialName("size_bytes") val sizeBytes: Long = 0,
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -109,8 +109,11 @@ private fun MiniCover(coverUrl: String, contentDescription: String) {
|
|||||||
* NowPlayingScreen via [onExpandClick].
|
* NowPlayingScreen via [onExpandClick].
|
||||||
*
|
*
|
||||||
* Layout (Column):
|
* Layout (Column):
|
||||||
* - Slim seek slider at the top (4dp track)
|
* - Slim seek slider pinned at the top (4dp track)
|
||||||
* - Row: cover | title/artist column | like | prev | play/pause | next
|
* - Row: cover | title/artist column | like | prev | play/pause | next.
|
||||||
|
* Weighted so it fills the rest of the fixed-height bar and centres its
|
||||||
|
* own content; otherwise the row keeps its intrinsic 48dp and the
|
||||||
|
* leftover height collects at the bottom as dead surface.
|
||||||
*
|
*
|
||||||
* No kebab on the mini bar (operator 2026-06-01): the full kebab
|
* No kebab on the mini bar (operator 2026-06-01): the full kebab
|
||||||
* surface lives on NowPlayingScreen, and dropping it from the mini
|
* surface lives on NowPlayingScreen, and dropping it from the mini
|
||||||
@@ -164,6 +167,12 @@ fun MiniPlayer(
|
|||||||
durationMs = state.durationMs,
|
durationMs = state.durationMs,
|
||||||
)
|
)
|
||||||
MiniRow(
|
MiniRow(
|
||||||
|
// Take whatever the progress fill leaves. Without this the
|
||||||
|
// Column stacks 4dp + the row's intrinsic 48dp from the top
|
||||||
|
// and the remaining 28dp of an 80dp bar sits empty
|
||||||
|
// underneath — the content looked top-aligned rather than
|
||||||
|
// centred, with a dead strip above the gesture bar.
|
||||||
|
modifier = Modifier.weight(1f),
|
||||||
track = track,
|
track = track,
|
||||||
isPlaying = state.isPlaying,
|
isPlaying = state.isPlaying,
|
||||||
isUpnpLoading = state.isUpnpLoading,
|
isUpnpLoading = state.isUpnpLoading,
|
||||||
@@ -205,6 +214,7 @@ private fun MiniProgressFill(positionMs: Long, durationMs: Long) {
|
|||||||
@Composable
|
@Composable
|
||||||
@Suppress("LongParameterList")
|
@Suppress("LongParameterList")
|
||||||
private fun MiniRow(
|
private fun MiniRow(
|
||||||
|
modifier: Modifier,
|
||||||
track: TrackRef,
|
track: TrackRef,
|
||||||
isPlaying: Boolean,
|
isPlaying: Boolean,
|
||||||
isUpnpLoading: Boolean,
|
isUpnpLoading: Boolean,
|
||||||
@@ -216,7 +226,7 @@ private fun MiniRow(
|
|||||||
onToggleLike: () -> Unit,
|
onToggleLike: () -> Unit,
|
||||||
) {
|
) {
|
||||||
Row(
|
Row(
|
||||||
modifier = Modifier
|
modifier = modifier
|
||||||
.fillMaxWidth()
|
.fillMaxWidth()
|
||||||
.padding(horizontal = 12.dp),
|
.padding(horizontal = 12.dp),
|
||||||
verticalAlignment = Alignment.CenterVertically,
|
verticalAlignment = Alignment.CenterVertically,
|
||||||
|
|||||||
+17
-4
@@ -9,7 +9,7 @@ import com.fabledsword.minstrel.update.data.ApkInstaller
|
|||||||
import com.fabledsword.minstrel.update.data.InstallStage
|
import com.fabledsword.minstrel.update.data.InstallStage
|
||||||
import com.fabledsword.minstrel.update.data.UpdateRepository
|
import com.fabledsword.minstrel.update.data.UpdateRepository
|
||||||
import com.fabledsword.minstrel.update.data.isBusy
|
import com.fabledsword.minstrel.update.data.isBusy
|
||||||
import com.fabledsword.minstrel.update.data.isVersionNewer
|
import com.fabledsword.minstrel.update.data.isUpdateAvailable
|
||||||
import com.fabledsword.minstrel.update.data.message
|
import com.fabledsword.minstrel.update.data.message
|
||||||
import com.fabledsword.minstrel.update.data.stage
|
import com.fabledsword.minstrel.update.data.stage
|
||||||
import dagger.hilt.android.lifecycle.HiltViewModel
|
import dagger.hilt.android.lifecycle.HiltViewModel
|
||||||
@@ -37,6 +37,10 @@ sealed interface UpdateCheckResult {
|
|||||||
|
|
||||||
data class AboutUiState(
|
data class AboutUiState(
|
||||||
val installedVersion: String = BuildConfig.VERSION_NAME,
|
val installedVersion: String = BuildConfig.VERSION_NAME,
|
||||||
|
// The value the platform installs by, and therefore the one the update
|
||||||
|
// check must decide on. Held in state rather than read inline so a test
|
||||||
|
// can drive the comparison without a BuildConfig.
|
||||||
|
val installedCode: Long = BuildConfig.VERSION_CODE.toLong(),
|
||||||
val isChecking: Boolean = false,
|
val isChecking: Boolean = false,
|
||||||
val installStage: InstallStage = InstallStage.IDLE,
|
val installStage: InstallStage = InstallStage.IDLE,
|
||||||
val installMessage: String? = null,
|
val installMessage: String? = null,
|
||||||
@@ -45,8 +49,9 @@ data class AboutUiState(
|
|||||||
|
|
||||||
/**
|
/**
|
||||||
* Backs the About card's update controls. "Check for updates" calls
|
* Backs the About card's update controls. "Check for updates" calls
|
||||||
* [UpdateRepository.getLatest], compares versus the build's
|
* [UpdateRepository.getLatest], compares versus this build via
|
||||||
* VERSION_NAME via [isVersionNewer], and reports the terminal state.
|
* [isUpdateAvailable] — on the ordering key where the server reports one,
|
||||||
|
* on the name otherwise — and reports the terminal state.
|
||||||
* When an update is available, [install] downloads the APK via
|
* When an update is available, [install] downloads the APK via
|
||||||
* [ApkInstaller] and installs it — routing the user to the "install
|
* [ApkInstaller] and installs it — routing the user to the "install
|
||||||
* unknown apps" settings page first when that permission hasn't been
|
* unknown apps" settings page first when that permission hasn't been
|
||||||
@@ -66,9 +71,17 @@ class AboutCardViewModel @Inject constructor(
|
|||||||
viewModelScope.launch {
|
viewModelScope.launch {
|
||||||
internal.update { it.copy(isChecking = true, installMessage = null) }
|
internal.update { it.copy(isChecking = true, installMessage = null) }
|
||||||
val installed = internal.value.installedVersion
|
val installed = internal.value.installedVersion
|
||||||
|
val installedCode = internal.value.installedCode
|
||||||
val result = runCatching { repository.getLatest() }
|
val result = runCatching { repository.getLatest() }
|
||||||
.map { latest ->
|
.map { latest ->
|
||||||
if (isVersionNewer(latest.version, installed)) {
|
if (
|
||||||
|
isUpdateAvailable(
|
||||||
|
serverCode = latest.code,
|
||||||
|
serverName = latest.version,
|
||||||
|
installedCode = installedCode,
|
||||||
|
installedName = installed,
|
||||||
|
)
|
||||||
|
) {
|
||||||
UpdateCheckResult.UpdateAvailable(latest)
|
UpdateCheckResult.UpdateAvailable(latest)
|
||||||
} else {
|
} else {
|
||||||
UpdateCheckResult.Latest
|
UpdateCheckResult.Latest
|
||||||
|
|||||||
+10
-2
@@ -19,7 +19,8 @@ private const val POLL_INTERVAL_MS = 24 * 60 * 60 * 1000L
|
|||||||
/**
|
/**
|
||||||
* Drives the shell's soft "update available" banner. Polls
|
* Drives the shell's soft "update available" banner. Polls
|
||||||
* `/api/client/version` at launch + every 24h and, when the bundled
|
* `/api/client/version` at launch + every 24h and, when the bundled
|
||||||
* APK is strictly newer than this build, exposes its [UpdateInfo] so
|
* APK outranks this build — by ordering key where the server reports one,
|
||||||
|
* by name otherwise — exposes its [UpdateInfo] so
|
||||||
* [com.fabledsword.minstrel.update.ui.UpdateBanner] can nudge an
|
* [com.fabledsword.minstrel.update.ui.UpdateBanner] can nudge an
|
||||||
* install. Mirrors Flutter's `ClientUpdateController`.
|
* install. Mirrors Flutter's `ClientUpdateController`.
|
||||||
*
|
*
|
||||||
@@ -58,6 +59,13 @@ class UpdateBannerController @Inject constructor(
|
|||||||
|
|
||||||
private suspend fun runOnce() {
|
private suspend fun runOnce() {
|
||||||
val info = runCatching { repository.getLatest() }.getOrNull() ?: return
|
val info = runCatching { repository.getLatest() }.getOrNull() ?: return
|
||||||
latest.value = info.takeIf { isVersionNewer(it.version, BuildConfig.VERSION_NAME) }
|
latest.value = info.takeIf {
|
||||||
|
isUpdateAvailable(
|
||||||
|
serverCode = it.code,
|
||||||
|
serverName = it.version,
|
||||||
|
installedCode = BuildConfig.VERSION_CODE.toLong(),
|
||||||
|
installedName = BuildConfig.VERSION_NAME,
|
||||||
|
)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -21,10 +21,39 @@ class UpdateRepository @Inject constructor(retrofit: Retrofit) {
|
|||||||
|
|
||||||
private fun UpdateInfoWire.toDomain(): UpdateInfo = UpdateInfo(
|
private fun UpdateInfoWire.toDomain(): UpdateInfo = UpdateInfo(
|
||||||
version = version,
|
version = version,
|
||||||
|
code = code,
|
||||||
|
channel = channel,
|
||||||
apkUrl = apkUrl,
|
apkUrl = apkUrl,
|
||||||
sizeBytes = sizeBytes,
|
sizeBytes = sizeBytes,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
/**
|
||||||
|
* True when [server] should be offered over the installed build.
|
||||||
|
*
|
||||||
|
* **Decide on the ordering key whenever the server sends one.** That is the
|
||||||
|
* same value Android's package installer compares, so an offer made this way
|
||||||
|
* implies an install the platform will actually accept. The app used to
|
||||||
|
* compare NAMES while the platform installed by `versionCode`, with nothing
|
||||||
|
* keeping the two orderings consistent — so it could offer a build Android
|
||||||
|
* then refused as a downgrade, or stay quiet about one it would have taken.
|
||||||
|
*
|
||||||
|
* Name comparison survives only as the fallback for a server that predates
|
||||||
|
* the field. A null code means "this server cannot tell me" — never "zero" —
|
||||||
|
* because treating absent as zero would rank every such server as infinitely
|
||||||
|
* old and offer its build to everyone, forever.
|
||||||
|
*/
|
||||||
|
fun isUpdateAvailable(
|
||||||
|
serverCode: Long?,
|
||||||
|
serverName: String,
|
||||||
|
installedCode: Long,
|
||||||
|
installedName: String,
|
||||||
|
): Boolean =
|
||||||
|
if (serverCode != null) {
|
||||||
|
serverCode > installedCode
|
||||||
|
} else {
|
||||||
|
isVersionNewer(serverName, installedName)
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* True when [server] is strictly newer than [installed]. Mirrors
|
* True when [server] is strictly newer than [installed]. Mirrors
|
||||||
* Flutter's `isVersionNewer` — splits both strings on `.`, parses
|
* Flutter's `isVersionNewer` — splits both strings on `.`, parses
|
||||||
|
|||||||
+163
@@ -0,0 +1,163 @@
|
|||||||
|
package com.fabledsword.minstrel.update.data
|
||||||
|
|
||||||
|
import org.junit.jupiter.api.Test
|
||||||
|
import kotlin.test.assertFalse
|
||||||
|
import kotlin.test.assertTrue
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The update channel had no tests at all before this. That is worth saying
|
||||||
|
* out loud, because the thing it decides — whether anyone is ever offered an
|
||||||
|
* update — fails silently in both directions: an update nobody is offered
|
||||||
|
* looks exactly like being up to date, and nobody files a bug about a prompt
|
||||||
|
* they never saw.
|
||||||
|
*/
|
||||||
|
class UpdateVersioningTest {
|
||||||
|
@Test
|
||||||
|
fun `decides on the ordering key when the server reports one`() {
|
||||||
|
assertTrue(
|
||||||
|
isUpdateAvailable(
|
||||||
|
serverCode = 3523847, serverName = "2026.09.10.1432",
|
||||||
|
installedCode = 3519456, installedName = "2026.09.09.1828",
|
||||||
|
),
|
||||||
|
)
|
||||||
|
assertFalse(
|
||||||
|
isUpdateAvailable(
|
||||||
|
serverCode = 3519456, serverName = "2026.09.09.1828",
|
||||||
|
installedCode = 3523847, installedName = "2026.09.10.1432",
|
||||||
|
),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `an equal ordering key is not an update`() {
|
||||||
|
assertFalse(
|
||||||
|
isUpdateAvailable(
|
||||||
|
serverCode = 3523847, serverName = "2026.09.10.1432",
|
||||||
|
installedCode = 3523847, installedName = "2026.09.10.1432",
|
||||||
|
),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The property the whole rework exists for: the offer must agree with what
|
||||||
|
* the platform will actually install. Where the two disagree, the ordering
|
||||||
|
* key wins, because that is the value Android compares.
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
fun `the ordering key wins even when the name disagrees`() {
|
||||||
|
// Name looks older, key is newer — e.g. an older commit rebuilt later.
|
||||||
|
assertTrue(
|
||||||
|
isUpdateAvailable(
|
||||||
|
serverCode = 9_000_000, serverName = "2020.01.01.0000",
|
||||||
|
installedCode = 1, installedName = "2099.12.31.2359",
|
||||||
|
),
|
||||||
|
)
|
||||||
|
// Name looks newer, key is not. Offering this would be offering an
|
||||||
|
// install the platform then refuses as a downgrade.
|
||||||
|
assertFalse(
|
||||||
|
isUpdateAvailable(
|
||||||
|
serverCode = 1, serverName = "2099.12.31.2359",
|
||||||
|
installedCode = 9_000_000, installedName = "2020.01.01.0000",
|
||||||
|
),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `falls back to the name when the server reports no ordering key`() {
|
||||||
|
assertTrue(
|
||||||
|
isUpdateAvailable(
|
||||||
|
serverCode = null, serverName = "2026.09.10.1432",
|
||||||
|
installedCode = 3519456, installedName = "2026.09.09.1828",
|
||||||
|
),
|
||||||
|
)
|
||||||
|
assertFalse(
|
||||||
|
isUpdateAvailable(
|
||||||
|
serverCode = null, serverName = "2026.09.09.1828",
|
||||||
|
installedCode = 3519456, installedName = "2026.09.10.1432",
|
||||||
|
),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A null code must never be read as zero. Zero would rank every
|
||||||
|
* older server as infinitely behind and offer its build to everyone,
|
||||||
|
* forever — so this asserts the fallback runs instead of a comparison
|
||||||
|
* against 0 succeeding by accident.
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
fun `a null ordering key is absent, not zero`() {
|
||||||
|
// installedCode is 0 here: if null coerced to 0, "0 > 0" would be
|
||||||
|
// false and this would wrongly report no update despite a newer name.
|
||||||
|
assertTrue(
|
||||||
|
isUpdateAvailable(
|
||||||
|
serverCode = null, serverName = "2026.09.10.1432",
|
||||||
|
installedCode = 0, installedName = "2026.09.09.1828",
|
||||||
|
),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The recorded migration constraint, pinned so it cannot be forgotten:
|
||||||
|
* the old scheme's fourth segment was a commit count (~1895), the new
|
||||||
|
* one is HHMM. Across a day boundary the date decides and all is well.
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
fun `new-scheme name outranks an old-scheme name on a later day`() {
|
||||||
|
assertTrue(isVersionNewer("2026.09.10.1432", "2026.09.09.1895"))
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* ...but on the SAME day the comparison comes down to HHMM against a
|
||||||
|
* commit count, and any build before ~19:00 UTC reads as older. This is
|
||||||
|
* why the first new-scheme release had to be cut on a later calendar day.
|
||||||
|
* Asserting the trap so nobody "fixes" it by accident.
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
fun `same-day new-scheme name can read older than an old-scheme name`() {
|
||||||
|
assertFalse(isVersionNewer("2026.09.09.1828", "2026.09.09.1895"))
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `name comparison degrades per segment rather than discarding`() {
|
||||||
|
// The string is still compared rather than rejected outright: an
|
||||||
|
// earlier segment decides and the unparseable tail never matters.
|
||||||
|
assertTrue(isVersionNewer("2026.09.10.1432-dev", "2026.09.09.1828"))
|
||||||
|
|
||||||
|
// A shorter name pads with zeros instead of being refused.
|
||||||
|
assertTrue(isVersionNewer("2026.09.10", "2026.09.09.9999"))
|
||||||
|
assertFalse(isVersionNewer("2026.09.10", "2026.09.10.0"))
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* What "costs that segment's precision" actually means, and it is worth
|
||||||
|
* pinning because it is a real edge rather than a nicety: when the
|
||||||
|
* unparseable segment is the DECIDING one, it reads as 0 and loses. So a
|
||||||
|
* `-dev` suffixed build compares as older than an unsuffixed one from the
|
||||||
|
* same minute.
|
||||||
|
*
|
||||||
|
* That is the correct behaviour for a degrading parser — it is bounded
|
||||||
|
* loss rather than a discarded string — but it is exactly why the channel
|
||||||
|
* belongs in its own field and never in the name.
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
fun `an unparseable deciding segment reads as zero and loses`() {
|
||||||
|
assertFalse(isVersionNewer("2026.09.10.1432-dev", "2026.09.10.1000"))
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Both sides unparseable (branch-name builds) falls back to string
|
||||||
|
* inequality, so a dev build still surfaces rather than comparing equal
|
||||||
|
* and going silent.
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
fun `two unparseable names fall back to string inequality`() {
|
||||||
|
assertTrue(isVersionNewer("main", "dev"))
|
||||||
|
assertFalse(isVersionNewer("dev", "dev"))
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `a leading v is ignored on either side`() {
|
||||||
|
assertTrue(isVersionNewer("v2026.09.10.1432", "2026.09.09.1828"))
|
||||||
|
assertFalse(isVersionNewer("v2026.09.10.1432", "v2026.09.10.1432"))
|
||||||
|
}
|
||||||
|
}
|
||||||
Binary file not shown.
+20
-47
@@ -62,57 +62,30 @@ None.
|
|||||||
- **Go toolchain pin.** `go.mod` is on `go 1.25.0` because `golang.org/x/crypto v0.51.0` declares 1.25 as its minimum. `ci-go:1.26` satisfies this with headroom. Future `x/crypto` bumps that move the Go floor should be paired with an image-tag bump in this file + the workflows.
|
- **Go toolchain pin.** `go.mod` is on `go 1.25.0` because `golang.org/x/crypto v0.51.0` declares 1.25 as its minimum. `ci-go:1.26` satisfies this with headroom. Future `x/crypto` bumps that move the Go floor should be paired with an image-tag bump in this file + the workflows.
|
||||||
- **In-app update channel — `needs:`, not polling.** `release.yml`'s `image-release` job declares `needs: [android-release]`, so on tag pushes the signed APK is guaranteed present before the image build starts — no polling window, no race. (The old cross-workflow polling against `flutter.yml` is gone with that workflow.) On non-tag `main` pushes `android-release` is skipped and `image-release` instead pulls the most recent release's APK and reconstructs its exact `versionName`, so `:latest` never ships without an update channel. It degrades to an empty `client/` — never a wrong version — if no release, asset, or tag commit-count can be resolved.
|
- **In-app update channel — `needs:`, not polling.** `release.yml`'s `image-release` job declares `needs: [android-release]`, so on tag pushes the signed APK is guaranteed present before the image build starts — no polling window, no race. (The old cross-workflow polling against `flutter.yml` is gone with that workflow.) On non-tag `main` pushes `android-release` is skipped and `image-release` instead pulls the most recent release's APK and reconstructs its exact `versionName`, so `:latest` never ships without an update channel. It degrades to an empty `client/` — never a wrong version — if no release, asset, or tag commit-count can be resolved.
|
||||||
- **Cache server reachability.** `test-web.yml` does NOT use `cache: 'npm'` on `actions/setup-node` — the Gitea Actions cache server isn't reachable from this runner's container network and `setup-node` was burning ~4m41s on ETIMEDOUT before failing open. With the migration to `ci-go:1.26`, `setup-node` is removed entirely (Node is in the image). The cache concern reappears if a future change re-introduces a network-dependent action.
|
- **Cache server reachability.** `test-web.yml` does NOT use `cache: 'npm'` on `actions/setup-node` — the Gitea Actions cache server isn't reachable from this runner's container network and `setup-node` was burning ~4m41s on ETIMEDOUT before failing open. With the migration to `ci-go:1.26`, `setup-node` is removed entirely (Node is in the image). The cache concern reappears if a future change re-introduces a network-dependent action.
|
||||||
- **Artifacts — use the mirrored actions, never `actions/{upload,download}-artifact`.**
|
- **Artifacts — stock `actions/upload-artifact@v7` and `actions/download-artifact@v8`; never `@v3`.**
|
||||||
```yaml
|
```yaml
|
||||||
uses: https://git.fabledsword.com/bvandeusen/upload-artifact@cb8afe72b42edc798abfb8fcb556cf660d894245
|
uses: actions/upload-artifact@v7
|
||||||
uses: https://git.fabledsword.com/bvandeusen/download-artifact@8d4e9521a5f7e5f8b6351f341f719f9f45a92a3a
|
uses: actions/download-artifact@v8
|
||||||
```
|
```
|
||||||
Upstream's `@v4+` cannot work against this instance and no server-side change
|
Stock works on this forge since the runner moved to gitea/runner 3.x, which
|
||||||
will help: `isGhes()` rejects any hostname that isn't `github.com` /
|
edits the actions' client-side `isGhes()` refusal out of their bundles. Proven
|
||||||
`*.ghe.com` / `*.localhost` and throws before it opens a connection, so the
|
on 2026-09-10 for upload v4–v7 and download v4–v8 (Scribe spike #3843). Until
|
||||||
server is never asked what it supports. `@v3` is worse — it reports success,
|
then this repo pinned SHA mirrors of the Forgejo project's forks, because
|
||||||
and Gitea then serves artifacts back only through the v4 API
|
upstream threw on the hostname before it opened a connection (Scribe 2255).
|
||||||
(`content_encoding = application/zip`), so a v3 upload is stored but invisible
|
|
||||||
to every retrieval path. A green job producing nothing retrievable; that is how
|
|
||||||
72 unreachable artifacts accumulated on this repo. Scribe issues 2255 / 2270.
|
|
||||||
|
|
||||||
Both are pull mirrors of the Forgejo project's forks
|
`@v3` is still broken: it reports success, and Gitea serves artifacts back only
|
||||||
(`code.forgejo.org/forgejo/{upload,download}-artifact`, one commit on upstream
|
through the v4 API (`content_encoding = application/zip`), so a v3 upload is
|
||||||
disabling that check), mirrored so CI depends on commits we hold and pinned by
|
stored but invisible to every retrieval path. That is how 72 unreachable
|
||||||
SHA because the mirrors auto-sync every 8h — a moved upstream tag would
|
artifacts accumulated on this repo (Scribe 2270).
|
||||||
otherwise silently change what runs.
|
|
||||||
|
|
||||||
**Match the pins on `@actions/artifact`, not on the actions' own version
|
**Pairing no longer needs managing.** This entry used to pin upload v5 against
|
||||||
numbers.** The two actions release on unrelated cadences, so equal version
|
download v6 so both bundled `@actions/artifact` ^4.0.0, warning that a mismatch
|
||||||
numbers do NOT mean a compatible pair — upload `v5` bundles `@actions/artifact`
|
across `release.yml`'s producer/consumer pair would list empty. Tested, and not
|
||||||
^4.0.0 while download `v5` bundles ^2.3.2. The pins above are upload **v5** and
|
true on this instance: every download major v4–v8 read the artifacts of every
|
||||||
download **v6**, which is the pairing that puts ^4.0.0 on both sides. This
|
upload major v4–v7, by name and by pattern (CI-runner run 6312). The only real
|
||||||
matters because `release.yml` is a producer/consumer pair — `android-release`
|
protocol break is v3 → v4. node24 is no longer a concern either — every
|
||||||
uploads `minstrel-apk`, `image-release` downloads it — and a protocol mismatch
|
CI-runner image carries Node 24 and the runner runs actions with the image's
|
||||||
across it yields an empty listing rather than an error, exactly the silent
|
`node`.
|
||||||
failure this entry exists to prevent.
|
|
||||||
|
|
||||||
| tag | `@actions/artifact` | runtime |
|
|
||||||
|---|---|---|
|
|
||||||
| upload v4 | ^2.1.1 | node20 |
|
|
||||||
| **upload v5** ← pinned | **^4.0.0** | node20 |
|
|
||||||
| download v4 | ^2.1.1 | node20 |
|
|
||||||
| download v5 | ^2.3.2 | node20 |
|
|
||||||
| **download v6** ← pinned | **^4.0.0** | node20 |
|
|
||||||
| download v7 | ^5.0.0 | **node24** |
|
|
||||||
|
|
||||||
The only true protocol break in this history was **v3 → v4** (upstream:
|
|
||||||
"Downloading artifacts that were created from `actions/upload-artifact@v3` and
|
|
||||||
below are not supported"); v4-and-up are one family. Later majors are mostly
|
|
||||||
ergonomics and runtime — upload v4 forbids re-uploading a name and caps a job
|
|
||||||
at 500 artifacts; download v5 made by-ID extraction match by-name.
|
|
||||||
|
|
||||||
**Do not jump the download pin to v7.** That major is a runner requirement, not
|
|
||||||
a feature change: it moves to `runs.using: node24` and upstream states it
|
|
||||||
"requires a minimum Actions Runner version of 2.327.1 … if you are using
|
|
||||||
self-hosted runners, ensure they are updated before upgrading." act_runner is
|
|
||||||
not GitHub's runner and makes no such version claim, so node24 is unverified
|
|
||||||
here. Everything currently pinned is node20.
|
|
||||||
|
|
||||||
Upload steps set `if-no-files-found: error` rather than the default `warn`, so
|
Upload steps set `if-no-files-found: error` rather than the default `warn`, so
|
||||||
an upload that matches nothing fails its own job instead of failing the
|
an upload that matches nothing fails its own job instead of failing the
|
||||||
|
|||||||
Executable
+159
@@ -0,0 +1,159 @@
|
|||||||
|
#!/usr/bin/env bash
|
||||||
|
#
|
||||||
|
# Derives the three values a build is stamped with, and the tag that names it.
|
||||||
|
#
|
||||||
|
# name=YYYY.MM.DD.HHMM label for people, from the timestamp of the newest
|
||||||
|
# commit that CHANGED SOMETHING SHIPPED (see SHIPPED)
|
||||||
|
# code=<int> ordering key, minutes since 2020-01-01 at BUILD time
|
||||||
|
# tag=v<name> what a release of this commit must be called
|
||||||
|
#
|
||||||
|
# Usage: ci/version.sh [<commit-ish>] (default HEAD)
|
||||||
|
#
|
||||||
|
# This exists as a script rather than inline workflow YAML for one reason:
|
||||||
|
# release.yml only runs on `main` and on tags, so anything living inside it is
|
||||||
|
# unverifiable until a release is already happening — which is the worst
|
||||||
|
# possible moment to discover the version is wrong, because the failure mode
|
||||||
|
# is silent (an update nobody is offered looks exactly like being current).
|
||||||
|
# As a script it can be executed by a test on every push instead.
|
||||||
|
#
|
||||||
|
# The two clocks are deliberate and are NOT interchangeable:
|
||||||
|
#
|
||||||
|
# The NAME answers "is this the same code?" — so it must read identically on
|
||||||
|
# every lane that builds this commit. Commit time does that; build time
|
||||||
|
# prints two different strings for one thing.
|
||||||
|
#
|
||||||
|
# The CODE answers "may this be installed over that?" — so it must be
|
||||||
|
# monotonic BY CONSTRUCTION. Build time is; commit time is not (rebuild an
|
||||||
|
# older commit and it goes down, which on a phone is a refused install), and
|
||||||
|
# a commit COUNT is worse still, because it runs ahead on `dev` and inverts
|
||||||
|
# against `main`.
|
||||||
|
set -euo pipefail
|
||||||
|
|
||||||
|
readonly EPOCH_2020=1577836800 # 2020-01-01T00:00:00Z
|
||||||
|
readonly REF="${1:-HEAD}"
|
||||||
|
|
||||||
|
# Both clocks are overridable so a test can pin them. Nothing but tests should
|
||||||
|
# set these — the defaults are the real derivation.
|
||||||
|
# The paths that do NOT ship, in either artifact. Everything else counts.
|
||||||
|
#
|
||||||
|
# A DENYLIST, and the direction is the whole point. As an allowlist, the list
|
||||||
|
# has to be updated by whoever adds a directory and nothing fails if they
|
||||||
|
# don't — so the failure mode is a changed artifact keeping its old version,
|
||||||
|
# silently, on a green run. That is a build lying about what it is. Inverted,
|
||||||
|
# new content counts by default and the only way to wrongly EXCLUDE something
|
||||||
|
# is to name it here deliberately.
|
||||||
|
#
|
||||||
|
# The two error directions are not symmetric, which is why this is not taste:
|
||||||
|
# wrongly excluded → changed artifact, unchanged version. A silent lie.
|
||||||
|
# wrongly included → version moves when nothing shipped. Cosmetic noise in
|
||||||
|
# a string nobody sorts.
|
||||||
|
#
|
||||||
|
# THIS REPO SHIPS TWO ARTIFACTS FROM ONE DERIVATION, and that is why the list
|
||||||
|
# is shorter than it looks like it should be. The server image ships cmd/,
|
||||||
|
# internal/, shared/, web/, config.example.yaml and client/; the APK ships
|
||||||
|
# android/. Neither ships the other's sources — but excluding android/ here
|
||||||
|
# would stop an Android-only commit from moving the APK's OWN version, which
|
||||||
|
# is the dangerous direction. So this is the union: exclude only what ships in
|
||||||
|
# NEITHER, and accept that an Android commit also nudges the server's reported
|
||||||
|
# version. Over-inclusion across the two, which is the harmless direction.
|
||||||
|
#
|
||||||
|
# The family's other repos (roundtable / roundtable-android) each keep a
|
||||||
|
# tighter list because they are separate repos with one artifact apiece. Do
|
||||||
|
# not copy theirs onto this one.
|
||||||
|
readonly SHIPPED=(
|
||||||
|
.
|
||||||
|
':!.gitea' # CI workflows — including this script's own caller
|
||||||
|
':!ci' # CI scripts — including this script
|
||||||
|
':!docs'
|
||||||
|
':!tools' # asset/font generators; their OUTPUT ships, they do not
|
||||||
|
':!deploy' # test-database bootstrap SQL
|
||||||
|
':!bin' # local `make build` output
|
||||||
|
':!*.md'
|
||||||
|
':!Makefile'
|
||||||
|
':!docker-compose.yml'
|
||||||
|
':!.env.example'
|
||||||
|
':!.gitignore'
|
||||||
|
':!.dockerignore'
|
||||||
|
':!renovate.json'
|
||||||
|
':!.golangci.yml'
|
||||||
|
|
||||||
|
# TESTS DO NOT SHIP, so they must not re-version an artifact.
|
||||||
|
#
|
||||||
|
# Named as globs rather than a directory because this repo has no tests/
|
||||||
|
# tree to exclude: Go tests sit inline beside the code they cover, and the
|
||||||
|
# web suite sits beside its modules. `go build` drops *_test.go outright and
|
||||||
|
# the Vite build never imports a .test.ts, so neither reaches an artifact.
|
||||||
|
#
|
||||||
|
# A commit touching a test AND its source still moves the version — the
|
||||||
|
# source path matches on its own. Only a test-ONLY commit is inert, which is
|
||||||
|
# the whole intent.
|
||||||
|
#
|
||||||
|
# Patterns match what exists today and nothing speculative: there are no
|
||||||
|
# .spec.* files, no __tests__/ directories and no androidTest/ tree. If any
|
||||||
|
# appear they will re-version until named here, which is the harmless
|
||||||
|
# direction and the reason this list is a denylist.
|
||||||
|
':!*_test.go' # 158 files, inline beside the code
|
||||||
|
':!*.test.ts' # 114 files
|
||||||
|
':!*.test.js'
|
||||||
|
':!android/app/src/test' # JVM unit tests; no androidTest tree exists
|
||||||
|
':!web/vitest.config.ts' # test-harness config, not build config
|
||||||
|
':!web/vitest.setup.ts'
|
||||||
|
)
|
||||||
|
|
||||||
|
commit_epoch="${MINSTREL_COMMIT_EPOCH:-}"
|
||||||
|
if [ -z "${commit_epoch}" ]; then
|
||||||
|
commit_epoch="$(git log --format=%ct -1 "${REF}" -- "${SHIPPED[@]}")"
|
||||||
|
# Loudly, on purpose. A silent fallback here is the landmine this whole
|
||||||
|
# script exists to avoid: a plausible-looking version that is quietly wrong,
|
||||||
|
# on a green run. Realistically this means a shallow clone (no commit in
|
||||||
|
# range touches the shipped set) rather than a repo of pure CI config.
|
||||||
|
if [ -z "${commit_epoch}" ]; then
|
||||||
|
echo "version.sh: no commit under '${REF}' touches the shipped file set — shallow clone? (needs fetch-depth: 0)" >&2
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
fi
|
||||||
|
now_epoch="${MINSTREL_NOW_EPOCH:-$(date -u +%s)}"
|
||||||
|
|
||||||
|
if ! name="$(date -u -d "@${commit_epoch}" +%Y.%m.%d.%H%M 2>/dev/null)"; then
|
||||||
|
echo "version.sh: could not read a commit timestamp from '${commit_epoch}'" >&2
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
|
||||||
|
if ! [ "${now_epoch}" -eq "${now_epoch}" ] 2>/dev/null; then
|
||||||
|
echo "version.sh: build timestamp '${now_epoch}' is not a number" >&2
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
code=$(( (now_epoch - EPOCH_2020) / 60 ))
|
||||||
|
|
||||||
|
# Assert the shape here, at the source. A malformed name builds, signs and
|
||||||
|
# publishes perfectly happily; it only surfaces later as an update channel
|
||||||
|
# that has quietly stopped offering anything.
|
||||||
|
if [[ ! "${name}" =~ ^[0-9]{4}\.[0-9]{2}\.[0-9]{2}\.[0-9]{4}$ ]]; then
|
||||||
|
echo "version.sh: name '${name}' is not YYYY.MM.DD.HHMM" >&2
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
|
||||||
|
# A non-positive key means the build clock is set before 2020, and every
|
||||||
|
# comparison downstream would be nonsense.
|
||||||
|
if [ "${code}" -le 0 ]; then
|
||||||
|
echo "version.sh: ordering key '${code}' is not positive — build clock wrong?" >&2
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
|
||||||
|
# Android's versionCode is a signed 32-bit int and the platform refuses an APK
|
||||||
|
# whose code exceeds it. At ~525k minutes a year this is four thousand years
|
||||||
|
# away in normal operation, so the realistic cause is a build machine with a
|
||||||
|
# badly wrong clock — which produces a code that is not merely too large but
|
||||||
|
# also unreachably high, permanently blocking every real build that follows
|
||||||
|
# from ever outranking it. Cheaper to refuse the build than to discover that
|
||||||
|
# from a phone that will not update.
|
||||||
|
readonly VERSION_CODE_CEILING=2147483647
|
||||||
|
if [ "${code}" -gt "${VERSION_CODE_CEILING}" ]; then
|
||||||
|
echo "version.sh: ordering key '${code}' exceeds versionCode's int32 ceiling — build clock wrong?" >&2
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
|
||||||
|
# KEY=VALUE, which is also exactly $GITHUB_OUTPUT's format.
|
||||||
|
echo "name=${name}"
|
||||||
|
echo "code=${code}"
|
||||||
|
echo "tag=v${name}"
|
||||||
@@ -97,14 +97,16 @@ type tuningSnapshot struct {
|
|||||||
func (h *handlers) tuningSnapshot() tuningSnapshot {
|
func (h *handlers) tuningSnapshot() tuningSnapshot {
|
||||||
var out tuningSnapshot
|
var out tuningSnapshot
|
||||||
out.Profiles = map[string]weightsResp{
|
out.Profiles = map[string]weightsResp{
|
||||||
recsettings.ScopeRadio: weightsRespFrom(h.recSettings.Weights(recsettings.ScopeRadio)),
|
recsettings.ScopeRadio: weightsRespFrom(h.recSettings.Weights(recsettings.ScopeRadio)),
|
||||||
recsettings.ScopeDailyMix: weightsRespFrom(h.recSettings.Weights(recsettings.ScopeDailyMix)),
|
recsettings.ScopeDailyMix: weightsRespFrom(h.recSettings.Weights(recsettings.ScopeDailyMix)),
|
||||||
|
recsettings.ScopeSongsLike: weightsRespFrom(h.recSettings.Weights(recsettings.ScopeSongsLike)),
|
||||||
}
|
}
|
||||||
out.Taste = tasteRespFrom(h.recSettings.Taste())
|
out.Taste = tasteRespFrom(h.recSettings.Taste())
|
||||||
out.Discover = discoverRespFrom(h.recSettings.Discover())
|
out.Discover = discoverRespFrom(h.recSettings.Discover())
|
||||||
out.Shipped.Profiles = map[string]weightsResp{
|
out.Shipped.Profiles = map[string]weightsResp{
|
||||||
recsettings.ScopeRadio: weightsRespFrom(recsettings.ShippedRadioWeights()),
|
recsettings.ScopeRadio: weightsRespFrom(recsettings.ShippedRadioWeights()),
|
||||||
recsettings.ScopeDailyMix: weightsRespFrom(recsettings.ShippedDailyMixWeights()),
|
recsettings.ScopeDailyMix: weightsRespFrom(recsettings.ShippedDailyMixWeights()),
|
||||||
|
recsettings.ScopeSongsLike: weightsRespFrom(recsettings.ShippedSongsLikeWeights()),
|
||||||
}
|
}
|
||||||
out.Shipped.Taste = tasteRespFrom(recsettings.ShippedTasteTuning())
|
out.Shipped.Taste = tasteRespFrom(recsettings.ShippedTasteTuning())
|
||||||
out.Shipped.Discover = discoverRespFrom(recsettings.ShippedDiscoverTuning())
|
out.Shipped.Discover = discoverRespFrom(recsettings.ShippedDiscoverTuning())
|
||||||
|
|||||||
+12
-6
@@ -24,6 +24,7 @@ import (
|
|||||||
"git.fabledsword.com/bvandeusen/minstrel/internal/playevents"
|
"git.fabledsword.com/bvandeusen/minstrel/internal/playevents"
|
||||||
"git.fabledsword.com/bvandeusen/minstrel/internal/playlists"
|
"git.fabledsword.com/bvandeusen/minstrel/internal/playlists"
|
||||||
"git.fabledsword.com/bvandeusen/minstrel/internal/reacquisition"
|
"git.fabledsword.com/bvandeusen/minstrel/internal/reacquisition"
|
||||||
|
"git.fabledsword.com/bvandeusen/minstrel/internal/recommendation"
|
||||||
"git.fabledsword.com/bvandeusen/minstrel/internal/recsettings"
|
"git.fabledsword.com/bvandeusen/minstrel/internal/recsettings"
|
||||||
"git.fabledsword.com/bvandeusen/minstrel/internal/tags"
|
"git.fabledsword.com/bvandeusen/minstrel/internal/tags"
|
||||||
"git.fabledsword.com/bvandeusen/minstrel/internal/tracks"
|
"git.fabledsword.com/bvandeusen/minstrel/internal/tracks"
|
||||||
@@ -55,6 +56,7 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev
|
|||||||
streamSecret: streamSecret,
|
streamSecret: streamSecret,
|
||||||
netSettings: netSettings,
|
netSettings: netSettings,
|
||||||
reacqSettings: reacqSettings,
|
reacqSettings: reacqSettings,
|
||||||
|
librarySize: recommendation.NewLibrarySize(nil),
|
||||||
}
|
}
|
||||||
|
|
||||||
r.Route("/api", func(api chi.Router) {
|
r.Route("/api", func(api chi.Router) {
|
||||||
@@ -268,12 +270,16 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev
|
|||||||
}
|
}
|
||||||
|
|
||||||
type handlers struct {
|
type handlers struct {
|
||||||
pool *pgxpool.Pool
|
pool *pgxpool.Pool
|
||||||
logger *slog.Logger
|
logger *slog.Logger
|
||||||
events *playevents.Writer
|
events *playevents.Writer
|
||||||
recCfg config.RecommendationConfig
|
recCfg config.RecommendationConfig
|
||||||
recSettings *recsettings.Service
|
recSettings *recsettings.Service
|
||||||
rng func() float64
|
rng func() float64
|
||||||
|
// librarySize memoises the track count that sizes the candidate pool
|
||||||
|
// (#3880). Held here rather than counted per request: the count is a
|
||||||
|
// full table scan, and library size only moves when a scan runs.
|
||||||
|
librarySize *recommendation.LibrarySize
|
||||||
lidarrCfg *lidarrconfig.Service
|
lidarrCfg *lidarrconfig.Service
|
||||||
lidarrRequests *lidarrrequests.Service
|
lidarrRequests *lidarrrequests.Service
|
||||||
lidarrQuarantine *lidarrquarantine.Service
|
lidarrQuarantine *lidarrquarantine.Service
|
||||||
|
|||||||
@@ -6,19 +6,21 @@ package api
|
|||||||
// /app/client/ at image build time.
|
// /app/client/ at image build time.
|
||||||
//
|
//
|
||||||
// Both endpoints are authenticated — the bandwidth cost of the APK
|
// Both endpoints are authenticated — the bandwidth cost of the APK
|
||||||
// (~30-60 MB) makes anonymous access an abuse vector. The Flutter
|
// (~30-60 MB) makes anonymous access an abuse vector. The client only
|
||||||
// client's polling only fires after login (banner mounts in the post-
|
// polls after login, so this gate is invisible to the actual update flow.
|
||||||
// login shell), so this gate is invisible to the actual update flow.
|
|
||||||
//
|
//
|
||||||
// /api/client/apk additionally rate-limits per user to a single
|
// /api/client/apk additionally rate-limits per user to a single
|
||||||
// download every 60s. Real install flows fire one download per
|
// download every 60s. Real install flows fire one download per
|
||||||
// update; anything tighter is scripted/abusive.
|
// update; anything tighter is scripted/abusive.
|
||||||
//
|
//
|
||||||
// Returns 404 gracefully when the APK isn't present (dev environments,
|
// Returns 404 gracefully when the APK isn't present (dev environments,
|
||||||
// pre-CI-wiring); the Flutter client treats 404 as "no update channel
|
// pre-CI-wiring); the client treats 404 as "no update channel available."
|
||||||
// available."
|
//
|
||||||
|
// (These paragraphs said "the Flutter client" until 2026-09-10. That client
|
||||||
|
// was deleted in v2026.08.18 — the Android app is the only one now.)
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"encoding/json"
|
||||||
"errors"
|
"errors"
|
||||||
"net/http"
|
"net/http"
|
||||||
"os"
|
"os"
|
||||||
@@ -84,8 +86,36 @@ func clientAPKAllowDownload(userID string, now time.Time) time.Duration {
|
|||||||
return 0
|
return 0
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// clientVersionSidecar is the JSON written beside the bundled APK by
|
||||||
|
// release.yml. It carries three values that are deliberately separate:
|
||||||
|
//
|
||||||
|
// - Name is a LABEL for people, "YYYY.MM.DD.HHMM" from the commit's
|
||||||
|
// timestamp. Two channels carrying the same code report the same name.
|
||||||
|
// - Code is the ORDERING KEY, minutes since 2020-01-01 at build time, and
|
||||||
|
// is the value Android itself installs by. It answers "may this be
|
||||||
|
// installed over that?" — the name never does.
|
||||||
|
// - Channel is a SIBLING FIELD, never a suffix inside the name.
|
||||||
|
//
|
||||||
|
// JSON rather than a positional line on purpose. The obvious growth path for
|
||||||
|
// the old one-value file was "<name> <code>", which a first-space split
|
||||||
|
// silently mangles the moment a third field appears: the code stops parsing,
|
||||||
|
// and the reader falls back to name comparison WITHOUT erroring.
|
||||||
|
type clientVersionSidecar struct {
|
||||||
|
Name string `json:"name"`
|
||||||
|
// Pointer, not int64: absent must stay distinguishable from zero. An
|
||||||
|
// artifact published before codes were recorded genuinely has no code —
|
||||||
|
// zero would claim it is infinitely old rather than unknown.
|
||||||
|
Code *int64 `json:"code"`
|
||||||
|
Channel string `json:"channel"`
|
||||||
|
}
|
||||||
|
|
||||||
type clientVersionResponse struct {
|
type clientVersionResponse struct {
|
||||||
Version string `json:"version"`
|
Version string `json:"version"`
|
||||||
|
// omitempty on both: the client must be able to tell "this server does
|
||||||
|
// not report a code" from "this build's code is 0", because those call
|
||||||
|
// for different behaviour on the other end.
|
||||||
|
Code *int64 `json:"code,omitempty"`
|
||||||
|
Channel string `json:"channel,omitempty"`
|
||||||
APKURL string `json:"apk_url"`
|
APKURL string `json:"apk_url"`
|
||||||
SizeBytes int64 `json:"size_bytes"`
|
SizeBytes int64 `json:"size_bytes"`
|
||||||
}
|
}
|
||||||
@@ -117,8 +147,25 @@ func (h *handlers) handleClientVersion(w http.ResponseWriter, _ *http.Request) {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
var sidecar clientVersionSidecar
|
||||||
|
if err := json.Unmarshal(versionBytes, &sidecar); err != nil {
|
||||||
|
// Fail LOUDLY rather than serving a blank version. The failure mode
|
||||||
|
// this avoids is the one that never gets reported: if an unreadable
|
||||||
|
// sidecar produced an empty name, every client would compare against
|
||||||
|
// nothing, conclude it was current, and go quiet — "I cannot read
|
||||||
|
// this" and "there is nothing newer" would be the same answer.
|
||||||
|
writeErrWithLog(w, h.logger, "client_version: sidecar is not valid JSON", err)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
if sidecar.Name == "" {
|
||||||
|
http.Error(w, `{"error":{"code":"bad_client_version","message":"version sidecar has no name"}}`, http.StatusInternalServerError)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
writeJSON(w, http.StatusOK, clientVersionResponse{
|
writeJSON(w, http.StatusOK, clientVersionResponse{
|
||||||
Version: strings.TrimSpace(string(versionBytes)),
|
Version: strings.TrimSpace(sidecar.Name),
|
||||||
|
Code: sidecar.Code,
|
||||||
|
Channel: strings.TrimSpace(sidecar.Channel),
|
||||||
APKURL: "/api/client/apk",
|
APKURL: "/api/client/apk",
|
||||||
SizeBytes: stat.Size(),
|
SizeBytes: stat.Size(),
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -77,18 +77,32 @@ func TestClientVersion_404WhenAPKButNoVersion(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestClientVersion_200WithBothFiles(t *testing.T) {
|
// writeClientAssets stages an APK plus a raw sidecar body, and returns the
|
||||||
|
// APK's size so callers can assert size_bytes without recomputing it.
|
||||||
|
func writeClientAssets(t *testing.T, sidecar string) int64 {
|
||||||
|
t.Helper()
|
||||||
dir := withClientAPKDir(t)
|
dir := withClientAPKDir(t)
|
||||||
body := []byte("fake apk content")
|
body := []byte("fake apk content")
|
||||||
if err := os.WriteFile(filepath.Join(dir, clientAPKFilename), body, 0o644); err != nil {
|
if err := os.WriteFile(filepath.Join(dir, clientAPKFilename), body, 0o644); err != nil {
|
||||||
t.Fatal(err)
|
t.Fatal(err)
|
||||||
}
|
}
|
||||||
if err := os.WriteFile(filepath.Join(dir, clientVersionFile), []byte("v2026.05.10\n"), 0o644); err != nil {
|
if err := os.WriteFile(filepath.Join(dir, clientVersionFile), []byte(sidecar), 0o644); err != nil {
|
||||||
t.Fatal(err)
|
t.Fatal(err)
|
||||||
}
|
}
|
||||||
|
return int64(len(body))
|
||||||
|
}
|
||||||
|
|
||||||
|
func getClientVersion(t *testing.T) *httptest.ResponseRecorder {
|
||||||
|
t.Helper()
|
||||||
h := &handlers{logger: slog.New(slog.NewTextHandler(io.Discard, nil))}
|
h := &handlers{logger: slog.New(slog.NewTextHandler(io.Discard, nil))}
|
||||||
rr := httptest.NewRecorder()
|
rr := httptest.NewRecorder()
|
||||||
h.handleClientVersion(rr, httptest.NewRequest(http.MethodGet, "/api/client/version", nil))
|
h.handleClientVersion(rr, httptest.NewRequest(http.MethodGet, "/api/client/version", nil))
|
||||||
|
return rr
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestClientVersion_200WithBothFiles(t *testing.T) {
|
||||||
|
size := writeClientAssets(t, `{"name":"2026.09.10.1432","code":3523847,"channel":"stable"}`+"\n")
|
||||||
|
rr := getClientVersion(t)
|
||||||
if rr.Code != http.StatusOK {
|
if rr.Code != http.StatusOK {
|
||||||
t.Fatalf("want 200, got %d (body: %s)", rr.Code, rr.Body.String())
|
t.Fatalf("want 200, got %d (body: %s)", rr.Code, rr.Body.String())
|
||||||
}
|
}
|
||||||
@@ -96,14 +110,69 @@ func TestClientVersion_200WithBothFiles(t *testing.T) {
|
|||||||
if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil {
|
if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil {
|
||||||
t.Fatal(err)
|
t.Fatal(err)
|
||||||
}
|
}
|
||||||
if resp.Version != "v2026.05.10" {
|
if resp.Version != "2026.09.10.1432" {
|
||||||
t.Errorf("version: want trimmed v2026.05.10, got %q", resp.Version)
|
t.Errorf("version: want 2026.09.10.1432, got %q", resp.Version)
|
||||||
|
}
|
||||||
|
if resp.Code == nil {
|
||||||
|
t.Fatal("code: want 3523847, got absent — the client decides on this, so absent means it silently falls back to name comparison")
|
||||||
|
}
|
||||||
|
if *resp.Code != 3523847 {
|
||||||
|
t.Errorf("code: want 3523847, got %d", *resp.Code)
|
||||||
|
}
|
||||||
|
if resp.Channel != "stable" {
|
||||||
|
t.Errorf("channel: want stable, got %q", resp.Channel)
|
||||||
}
|
}
|
||||||
if resp.APKURL != "/api/client/apk" {
|
if resp.APKURL != "/api/client/apk" {
|
||||||
t.Errorf("apk_url: want /api/client/apk, got %q", resp.APKURL)
|
t.Errorf("apk_url: want /api/client/apk, got %q", resp.APKURL)
|
||||||
}
|
}
|
||||||
if resp.SizeBytes != int64(len(body)) {
|
if resp.SizeBytes != size {
|
||||||
t.Errorf("size_bytes: want %d, got %d", len(body), resp.SizeBytes)
|
t.Errorf("size_bytes: want %d, got %d", size, resp.SizeBytes)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// A release published before ordering keys were recorded has a name and
|
||||||
|
// genuinely no code. That must arrive as ABSENT, not as 0 — zero would claim
|
||||||
|
// the build is infinitely old and offer an update to everyone forever.
|
||||||
|
func TestClientVersion_CodeAbsentIsOmittedNotZero(t *testing.T) {
|
||||||
|
writeClientAssets(t, `{"name":"2026.09.09","code":null,"channel":"stable"}`)
|
||||||
|
rr := getClientVersion(t)
|
||||||
|
if rr.Code != http.StatusOK {
|
||||||
|
t.Fatalf("want 200, got %d (body: %s)", rr.Code, rr.Body.String())
|
||||||
|
}
|
||||||
|
var resp clientVersionResponse
|
||||||
|
if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if resp.Code != nil {
|
||||||
|
t.Errorf("code: want absent, got %d", *resp.Code)
|
||||||
|
}
|
||||||
|
// The wire must omit the key entirely, so a client can distinguish
|
||||||
|
// "this server reports no code" from "this build's code is 0".
|
||||||
|
var raw map[string]any
|
||||||
|
if err := json.Unmarshal(rr.Body.Bytes(), &raw); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if _, present := raw["code"]; present {
|
||||||
|
t.Errorf("code key should be omitted entirely, body was %s", rr.Body.String())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// The failure this guards is the one nobody reports: if an unreadable sidecar
|
||||||
|
// produced an empty version, every client would compare against nothing,
|
||||||
|
// decide it was current, and go quiet. "I cannot read this" and "there is
|
||||||
|
// nothing newer" must not be the same answer.
|
||||||
|
func TestClientVersion_MalformedSidecarErrorsRatherThanReportingNothing(t *testing.T) {
|
||||||
|
for _, sidecar := range []string{
|
||||||
|
"2026.09.10.1432", // the OLD plain-text format
|
||||||
|
`{"name":"x",`, // truncated JSON
|
||||||
|
`{"code":123,"channel":"dev"}`, // valid JSON, no name
|
||||||
|
"",
|
||||||
|
} {
|
||||||
|
writeClientAssets(t, sidecar)
|
||||||
|
rr := getClientVersion(t)
|
||||||
|
if rr.Code == http.StatusOK {
|
||||||
|
t.Errorf("sidecar %q: want an error status, got 200 with body %s", sidecar, rr.Body.String())
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+20
-2
@@ -87,7 +87,17 @@ func (h *handlers) handleRadio(w http.ResponseWriter, r *http.Request) {
|
|||||||
currentVec.DeviceClass = latestDeviceClass(r.Context(), q, user.ID, h.logger)
|
currentVec.DeviceClass = latestDeviceClass(r.Context(), q, user.ID, h.logger)
|
||||||
|
|
||||||
exclude := parseExcludeParam(r.URL.Query().Get("exclude"))
|
exclude := parseExcludeParam(r.URL.Query().Get("exclude"))
|
||||||
limits := recommendation.DefaultCandidateSourceLimits()
|
// Size the pool to the library (#3880). A fixed ~170 candidates samples a
|
||||||
|
// shrinking fraction of a growing collection, which is what made the
|
||||||
|
// recommendations feel less relevant as the library grew. Degrades to the
|
||||||
|
// base limits if the count is unavailable — never fails the request over a
|
||||||
|
// sizing hint.
|
||||||
|
librarySize := h.librarySize.Get(r.Context(), func(ctx context.Context) (int64, error) {
|
||||||
|
return recommendation.CountLibraryTracks(ctx, q)
|
||||||
|
})
|
||||||
|
limits := recommendation.ScaleForLibrary(
|
||||||
|
recommendation.DefaultCandidateSourceLimits(), librarySize,
|
||||||
|
)
|
||||||
candidates, err := recommendation.LoadCandidatesFromSimilarity(
|
candidates, err := recommendation.LoadCandidatesFromSimilarity(
|
||||||
r.Context(), q, user.ID, seedID,
|
r.Context(), q, user.ID, seedID,
|
||||||
h.recCfg.RecentlyPlayedHours, currentVec, exclude, limits,
|
h.recCfg.RecentlyPlayedHours, currentVec, exclude, limits,
|
||||||
@@ -108,7 +118,15 @@ func (h *handlers) handleRadio(w http.ResponseWriter, r *http.Request) {
|
|||||||
// Scoring weights come from the DB-backed tuning lab (#1250) —
|
// Scoring weights come from the DB-backed tuning lab (#1250) —
|
||||||
// read per request so an admin change takes effect live.
|
// read per request so an admin change takes effect live.
|
||||||
weights := h.recSettings.Weights(recsettings.ScopeRadio)
|
weights := h.recSettings.Weights(recsettings.ScopeRadio)
|
||||||
picks := recommendation.Shuffle(candidates, weights, time.Now().UTC(), h.rng, limit-1)
|
// Diversity caps (#3882). Radio had none while every sibling surface did,
|
||||||
|
// which is how a whole session could come back from one artist. Scaled to
|
||||||
|
// the requested length so a 20-track radio and a 200-track one are capped
|
||||||
|
// alike; Shuffle relaxes them rather than returning a short radio.
|
||||||
|
//
|
||||||
|
// limit-1 because the seed track occupies the first slot and is prepended
|
||||||
|
// below — the caps govern the tracks that FOLLOW it.
|
||||||
|
caps := recommendation.RadioDiversityCaps(limit - 1)
|
||||||
|
picks := recommendation.Shuffle(candidates, weights, time.Now().UTC(), h.rng, limit-1, caps)
|
||||||
|
|
||||||
out := make([]TrackRef, 0, len(picks)+1)
|
out := make([]TrackRef, 0, len(picks)+1)
|
||||||
out = append(out, trackRefFrom(track, album.Title, artist.Name))
|
out = append(out, trackRefFrom(track, album.Title, artist.Name))
|
||||||
|
|||||||
@@ -0,0 +1,16 @@
|
|||||||
|
-- Drop the rows the narrower constraints are about to forbid, or re-adding
|
||||||
|
-- them fails against existing data (the 0051 down-migration pattern).
|
||||||
|
DELETE FROM recommendation_weight_profiles WHERE profile = 'songs_like';
|
||||||
|
DELETE FROM recommendation_tuning_audit WHERE scope = 'songs_like';
|
||||||
|
|
||||||
|
ALTER TABLE recommendation_tuning_audit
|
||||||
|
DROP CONSTRAINT recommendation_tuning_audit_scope_check;
|
||||||
|
ALTER TABLE recommendation_tuning_audit
|
||||||
|
ADD CONSTRAINT recommendation_tuning_audit_scope_check
|
||||||
|
CHECK (scope IN ('radio', 'daily_mix', 'taste', 'discover'));
|
||||||
|
|
||||||
|
ALTER TABLE recommendation_weight_profiles
|
||||||
|
DROP CONSTRAINT recommendation_weight_profiles_profile_check;
|
||||||
|
ALTER TABLE recommendation_weight_profiles
|
||||||
|
ADD CONSTRAINT recommendation_weight_profiles_profile_check
|
||||||
|
CHECK (profile IN ('radio', 'daily_mix'));
|
||||||
@@ -0,0 +1,37 @@
|
|||||||
|
-- 0057_songs_like_tuning.up.sql — a THIRD weight profile, for Songs-like
|
||||||
|
-- (Scribe #3881, milestone #398).
|
||||||
|
--
|
||||||
|
-- Songs-like shared the `daily_mix` profile with For-You, and that is the bug.
|
||||||
|
-- The two surfaces want opposite things: For-You is a broad "what will they
|
||||||
|
-- enjoy today", Songs-like answers "what sounds like THIS", and under one set
|
||||||
|
-- of weights the broad answer wins. Operator, 2026-09-10: "I'm expecting to
|
||||||
|
-- get a consistent sound and style from the experience... I was getting a
|
||||||
|
-- seeming wide variety of music from each one when I was hoping to stay in a
|
||||||
|
-- certain neighborhood."
|
||||||
|
--
|
||||||
|
-- Under the shared daily_mix weights, an UNRELATED track the user had liked
|
||||||
|
-- and not played recently scored 1.0 + 2.0 + 1.0 = 4.0 before taste, while a
|
||||||
|
-- PERFECT similarity match they had not liked scored 1.0 + 1.5 = 2.5. Liking
|
||||||
|
-- something outranked sounding like the seed. Splitting the profile is what
|
||||||
|
-- lets similarity dominate here without making For-You narrow.
|
||||||
|
--
|
||||||
|
-- Rows are seeded by the recsettings boot reconcile, not here, so shipped
|
||||||
|
-- defaults live in exactly one place (Go) — same as 0040.
|
||||||
|
|
||||||
|
-- Rule #36: a new value for a CHECK-gated column needs the constraint
|
||||||
|
-- rewritten in the SAME change, or the first row written under the new
|
||||||
|
-- profile fails at runtime rather than at migrate time.
|
||||||
|
ALTER TABLE recommendation_weight_profiles
|
||||||
|
DROP CONSTRAINT recommendation_weight_profiles_profile_check;
|
||||||
|
ALTER TABLE recommendation_weight_profiles
|
||||||
|
ADD CONSTRAINT recommendation_weight_profiles_profile_check
|
||||||
|
CHECK (profile IN ('radio', 'daily_mix', 'songs_like'));
|
||||||
|
|
||||||
|
-- The audit table gates the same name on a separate constraint. Missing this
|
||||||
|
-- one would let the profile be seeded and then fail on the first knob turn —
|
||||||
|
-- green at boot, 500 on first use.
|
||||||
|
ALTER TABLE recommendation_tuning_audit
|
||||||
|
DROP CONSTRAINT recommendation_tuning_audit_scope_check;
|
||||||
|
ALTER TABLE recommendation_tuning_audit
|
||||||
|
ADD CONSTRAINT recommendation_tuning_audit_scope_check
|
||||||
|
CHECK (scope IN ('radio', 'daily_mix', 'taste', 'discover', 'songs_like'));
|
||||||
+101
-15
@@ -219,7 +219,30 @@ var (
|
|||||||
// uniform with radio pending trend data.
|
// uniform with radio pending trend data.
|
||||||
ContextTimeWeight: 1.0,
|
ContextTimeWeight: 1.0,
|
||||||
}
|
}
|
||||||
|
// Songs-like's own profile (#3881). Pre-push literal only; shipped
|
||||||
|
// defaults live in recsettings.ShippedSongsLikeWeights and must stay in
|
||||||
|
// sync with it, exactly as systemMixWeights does above.
|
||||||
|
//
|
||||||
|
// SimilarityWeight dominates here and every seed-INDEPENDENT term is
|
||||||
|
// demoted, which is the whole difference between this surface and For-You.
|
||||||
|
// See ShippedSongsLikeWeights for the property the numbers encode.
|
||||||
|
songsLikeWeights = recommendation.ScoringWeights{
|
||||||
|
BaseWeight: 1.0,
|
||||||
|
LikeBoost: 0.5,
|
||||||
|
RecencyWeight: 0.25,
|
||||||
|
SkipPenalty: 2.0,
|
||||||
|
JitterMagnitude: 0.05,
|
||||||
|
ContextWeight: 0.5,
|
||||||
|
SimilarityWeight: 4.0,
|
||||||
|
TasteWeight: 0.25,
|
||||||
|
ContextTimeWeight: 0.5,
|
||||||
|
}
|
||||||
systemTasteConfig = taste.DefaultConfig()
|
systemTasteConfig = taste.DefaultConfig()
|
||||||
|
|
||||||
|
// Sizes the candidate pool to the library (#3880). Cached because the
|
||||||
|
// count is a full table scan and the daily build runs it once per user;
|
||||||
|
// within the TTL every user in a build shares one count.
|
||||||
|
systemLibrarySize = recommendation.NewLibrarySize(nil)
|
||||||
)
|
)
|
||||||
|
|
||||||
// SetSystemMixWeights installs the current daily_mix scoring weights.
|
// SetSystemMixWeights installs the current daily_mix scoring weights.
|
||||||
@@ -230,6 +253,20 @@ func SetSystemMixWeights(w recommendation.ScoringWeights) {
|
|||||||
systemMixWeights = w
|
systemMixWeights = w
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// SetSongsLikeWeights installs the songs_like scoring profile (#3881).
|
||||||
|
// Same push model as SetSystemMixWeights — recsettings calls it on boot and
|
||||||
|
// after every knob turn, so a tuning change takes effect on the next daily
|
||||||
|
// build with no restart.
|
||||||
|
//
|
||||||
|
// Separate from systemMixWeights because Songs-like and For-You want opposite
|
||||||
|
// things: For-You roams, Songs-like must not. Sharing one profile is what made
|
||||||
|
// "Songs like X" wander.
|
||||||
|
func SetSongsLikeWeights(w recommendation.ScoringWeights) {
|
||||||
|
systemTuningMu.Lock()
|
||||||
|
defer systemTuningMu.Unlock()
|
||||||
|
songsLikeWeights = w
|
||||||
|
}
|
||||||
|
|
||||||
// SetTasteConfig installs the taste-profile build configuration
|
// SetTasteConfig installs the taste-profile build configuration
|
||||||
// (half-life + engagement curve, #1250). Same push model as
|
// (half-life + engagement curve, #1250). Same push model as
|
||||||
// SetSystemMixWeights.
|
// SetSystemMixWeights.
|
||||||
@@ -239,6 +276,12 @@ func SetTasteConfig(c taste.Config) {
|
|||||||
systemTasteConfig = c
|
systemTasteConfig = c
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func currentSongsLikeWeights() recommendation.ScoringWeights {
|
||||||
|
systemTuningMu.RLock()
|
||||||
|
defer systemTuningMu.RUnlock()
|
||||||
|
return songsLikeWeights
|
||||||
|
}
|
||||||
|
|
||||||
func currentSystemMixWeights() recommendation.ScoringWeights {
|
func currentSystemMixWeights() recommendation.ScoringWeights {
|
||||||
systemTuningMu.RLock()
|
systemTuningMu.RLock()
|
||||||
defer systemTuningMu.RUnlock()
|
defer systemTuningMu.RUnlock()
|
||||||
@@ -392,7 +435,13 @@ func pickWeightedTail(tailPool []recommendation.Candidate, dateStr string, tailN
|
|||||||
// tieBreakHash). The scoring RNG is seeded by userIDHash so jitter is
|
// tieBreakHash). The scoring RNG is seeded by userIDHash so jitter is
|
||||||
// deterministic per (user, day) but rotates across days. Pure — no
|
// deterministic per (user, day) but rotates across days. Pure — no
|
||||||
// truncation, no cap.
|
// truncation, no cap.
|
||||||
func scoreAndSortCandidates(cands []recommendation.Candidate, userID pgtype.UUID, dateStr string, now time.Time) []recommendation.Candidate {
|
// weights is a parameter rather than a read of currentSystemMixWeights()
|
||||||
|
// because this sort IS the selection: the caller caps and truncates in the
|
||||||
|
// order this returns, so whatever profile ranks here decides which tracks
|
||||||
|
// reach the playlist. Scoring with daily_mix here and re-scoring with
|
||||||
|
// songs_like afterwards would have let the new profile relabel tracks it had
|
||||||
|
// no part in choosing — inert where it matters (#3881).
|
||||||
|
func scoreAndSortCandidates(cands []recommendation.Candidate, userID pgtype.UUID, dateStr string, now time.Time, weights recommendation.ScoringWeights) []recommendation.Candidate {
|
||||||
rng := rand.New(rand.NewSource(int64(userIDHash(userID, dateStr))))
|
rng := rand.New(rand.NewSource(int64(userIDHash(userID, dateStr))))
|
||||||
type scored struct {
|
type scored struct {
|
||||||
c recommendation.Candidate
|
c recommendation.Candidate
|
||||||
@@ -411,7 +460,6 @@ func scoreAndSortCandidates(cands []recommendation.Candidate, userID pgtype.UUID
|
|||||||
sort.SliceStable(ordered, func(i, j int) bool {
|
sort.SliceStable(ordered, func(i, j int) bool {
|
||||||
return uuidLessPL(ordered[i].Track.ID, ordered[j].Track.ID)
|
return uuidLessPL(ordered[i].Track.ID, ordered[j].Track.ID)
|
||||||
})
|
})
|
||||||
weights := currentSystemMixWeights()
|
|
||||||
pairs := make([]scored, len(ordered))
|
pairs := make([]scored, len(ordered))
|
||||||
for i, c := range ordered {
|
for i, c := range ordered {
|
||||||
pairs[i] = scored{c: c, score: recommendation.Score(c.Inputs, weights, now, rng.Float64)}
|
pairs[i] = scored{c: c, score: recommendation.Score(c.Inputs, weights, now, rng.Float64)}
|
||||||
@@ -632,6 +680,13 @@ func produceSeedMixes(
|
|||||||
seedPool := pickSeedArtistsFromRows(seedRowsLocal)
|
seedPool := pickSeedArtistsFromRows(seedRowsLocal)
|
||||||
seeds := pickSeedArtistsForDay(seedPool, userID, dateStr)
|
seeds := pickSeedArtistsForDay(seedPool, userID, dateStr)
|
||||||
|
|
||||||
|
// Once per build rather than once per seed artist — six mixes would
|
||||||
|
// otherwise mean six full-table counts for a number that cannot have
|
||||||
|
// changed between them.
|
||||||
|
librarySize := systemLibrarySize.Get(ctx, func(c context.Context) (int64, error) {
|
||||||
|
return recommendation.CountLibraryTracks(c, q)
|
||||||
|
})
|
||||||
|
|
||||||
out := make([]builtPlaylist, 0, len(seeds))
|
out := make([]builtPlaylist, 0, len(seeds))
|
||||||
for _, artistID := range seeds {
|
for _, artistID := range seeds {
|
||||||
artistRow, aerr := q.GetArtistByID(ctx, artistID)
|
artistRow, aerr := q.GetArtistByID(ctx, artistID)
|
||||||
@@ -647,23 +702,46 @@ func produceSeedMixes(
|
|||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
zeroVec := recommendation.SessionVector{Seed: true}
|
zeroVec := recommendation.SessionVector{Seed: true}
|
||||||
|
// Songs-like's own pool shape, not the default (#3881). Same total
|
||||||
|
// size; the composition shifts toward arms that actually measure
|
||||||
|
// distance from the seed. The default gave ~29% of candidates a
|
||||||
|
// sim_score of literally 0.
|
||||||
|
//
|
||||||
|
// Then scaled to the library (#3880): a bigger collection should put
|
||||||
|
// more genuinely-similar candidates in reach, not the same ~170
|
||||||
|
// regardless. The weights still rank sim_score-0 arms last, so the
|
||||||
|
// growth reaches coherence rather than working against it.
|
||||||
cands, cerr := recommendation.LoadCandidatesFromSimilarity(
|
cands, cerr := recommendation.LoadCandidatesFromSimilarity(
|
||||||
ctx, q, userID, seedTrack, 1, zeroVec, []pgtype.UUID{seedTrack},
|
ctx, q, userID, seedTrack, 1, zeroVec, []pgtype.UUID{seedTrack},
|
||||||
recommendation.DefaultCandidateSourceLimits(),
|
recommendation.ScaleForLibrary(
|
||||||
|
recommendation.SongsLikeCandidateSourceLimits(), librarySize,
|
||||||
|
),
|
||||||
)
|
)
|
||||||
if cerr != nil {
|
if cerr != nil {
|
||||||
logger.Warn("system playlist: seed candidates load failed; skipping",
|
logger.Warn("system playlist: seed candidates load failed; skipping",
|
||||||
"artist_id", uuidStringPL(artistID), "err", cerr)
|
"artist_id", uuidStringPL(artistID), "err", cerr)
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
// "Songs like X" excludes X's own songs.
|
// The seed artist's own songs are ELIGIBLE here, deliberately.
|
||||||
filtered := make([]recommendation.Candidate, 0, len(cands))
|
//
|
||||||
for _, c := range cands {
|
// This used to filter them out — "Songs like X excludes X's own
|
||||||
if !pgtypeUUIDEqual(c.Track.ArtistID, artistID) {
|
// songs" — which reads as obviously right and is not. The seed is a
|
||||||
filtered = append(filtered, c)
|
// TRACK, and the tracks most likely to sound like it are usually the
|
||||||
}
|
// rest of that artist's catalogue; excluding them threw away the
|
||||||
}
|
// nearest neighbours of the very thing the mix is built around, and
|
||||||
tracks := pickTopN(filtered, userID, dateStr, now, systemMixLength)
|
// then reached further out to replace them. On a surface whose whole
|
||||||
|
// job is staying in one neighbourhood, that is backwards.
|
||||||
|
//
|
||||||
|
// Operator, 2026-09-10: "it should also be able to include music from
|
||||||
|
// the same artist."
|
||||||
|
//
|
||||||
|
// Domination is bounded by the cap rather than by exclusion, which is
|
||||||
|
// the distinction that makes this safe: capCandidatesByAlbumAndArtist
|
||||||
|
// inside pickTopN allows at most discoverMaxTracksPerArtist (3) of a
|
||||||
|
// 25-track mix — 12%, a presence rather than a takeover. The seed
|
||||||
|
// track itself still cannot appear; it is passed as an exclusion to
|
||||||
|
// LoadCandidatesFromSimilarity above.
|
||||||
|
tracks := pickTopN(cands, userID, dateStr, now, systemMixLength)
|
||||||
if len(tracks) == 0 {
|
if len(tracks) == 0 {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
@@ -838,13 +916,20 @@ func BuildSystemPlaylists(ctx context.Context, pool *pgxpool.Pool, logger *slog.
|
|||||||
// truncates to n. Used by Songs-like-X (and as the fallback inside
|
// truncates to n. Used by Songs-like-X (and as the fallback inside
|
||||||
// pickHeadAndTail for small pools).
|
// pickHeadAndTail for small pools).
|
||||||
func pickTopN(cands []recommendation.Candidate, userID pgtype.UUID, dateStr string, now time.Time, n int) []rankedCandidate {
|
func pickTopN(cands []recommendation.Candidate, userID pgtype.UUID, dateStr string, now time.Time, n int) []rankedCandidate {
|
||||||
sorted := scoreAndSortCandidates(cands, userID, dateStr, now)
|
// songs_like, not daily_mix (#3881). produceSeedMixes is this function's
|
||||||
|
// only caller, so the switch moves exactly one surface — For-You ranks
|
||||||
|
// through pickHeadAndTail and keeps the broader daily_mix profile.
|
||||||
|
//
|
||||||
|
// The SAME profile does the selection sort and the final score. Passing
|
||||||
|
// one and using the other is the subtle version of this bug: the playlist
|
||||||
|
// would still be chosen by daily_mix and merely wear songs_like numbers.
|
||||||
|
weights := currentSongsLikeWeights()
|
||||||
|
sorted := scoreAndSortCandidates(cands, userID, dateStr, now, weights)
|
||||||
capped := capCandidatesByAlbumAndArtist(sorted)
|
capped := capCandidatesByAlbumAndArtist(sorted)
|
||||||
if len(capped) > n {
|
if len(capped) > n {
|
||||||
capped = capped[:n]
|
capped = capped[:n]
|
||||||
}
|
}
|
||||||
rng := rand.New(rand.NewSource(int64(userIDHash(userID, dateStr))))
|
rng := rand.New(rand.NewSource(int64(userIDHash(userID, dateStr))))
|
||||||
weights := currentSystemMixWeights()
|
|
||||||
out := make([]rankedCandidate, len(capped))
|
out := make([]rankedCandidate, len(capped))
|
||||||
for i, c := range capped {
|
for i, c := range capped {
|
||||||
out[i] = rankedCandidate{
|
out[i] = rankedCandidate{
|
||||||
@@ -873,10 +958,11 @@ func pickHeadAndTail(
|
|||||||
cands []recommendation.Candidate, seedOf map[pgtype.UUID]int, numSeeds int,
|
cands []recommendation.Candidate, seedOf map[pgtype.UUID]int, numSeeds int,
|
||||||
userID pgtype.UUID, dateStr string, now time.Time, headN, tailN int,
|
userID pgtype.UUID, dateStr string, now time.Time, headN, tailN int,
|
||||||
) []rankedCandidate {
|
) []rankedCandidate {
|
||||||
sorted := scoreAndSortCandidates(cands, userID, dateStr, now)
|
// daily_mix — For-You is the broad surface and keeps the roaming profile.
|
||||||
|
weights := currentSystemMixWeights()
|
||||||
|
sorted := scoreAndSortCandidates(cands, userID, dateStr, now, weights)
|
||||||
capped := capCandidatesByAlbumAndArtist(sorted)
|
capped := capCandidatesByAlbumAndArtist(sorted)
|
||||||
rng := rand.New(rand.NewSource(int64(userIDHash(userID, dateStr))))
|
rng := rand.New(rand.NewSource(int64(userIDHash(userID, dateStr))))
|
||||||
weights := currentSystemMixWeights()
|
|
||||||
|
|
||||||
total := headN + tailN
|
total := headN + tailN
|
||||||
if len(capped) <= total {
|
if len(capped) <= total {
|
||||||
|
|||||||
@@ -2,8 +2,10 @@ package playlists_test
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"fmt"
|
||||||
"io"
|
"io"
|
||||||
"log/slog"
|
"log/slog"
|
||||||
|
"path/filepath"
|
||||||
"sync"
|
"sync"
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
"time"
|
||||||
@@ -287,6 +289,141 @@ func TestBuildSystemPlaylists_Concurrency(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// seedSharedArtistLibrary seeds artists that genuinely OWN several tracks.
|
||||||
|
//
|
||||||
|
// seedActiveLibrary cannot be used for this: its helper documents that
|
||||||
|
// "artist and album are not deduplicated across calls (mbid-less upsert)",
|
||||||
|
// so every track gets its own artist row and a seed artist always has
|
||||||
|
// exactly one track — the seed itself, which is excluded. A same-artist
|
||||||
|
// assertion against that fixture can never pass no matter what the code
|
||||||
|
// does, which is how the first version of this test failed.
|
||||||
|
//
|
||||||
|
// Albums are not deduplicated either, for the same mbid-less reason, so each
|
||||||
|
// track ends up under its own album row however the titles are written. That
|
||||||
|
// is convenient here rather than a problem: it means the per-ALBUM cap (2)
|
||||||
|
// never binds, and the per-ARTIST cap (3) is unambiguously the thing under
|
||||||
|
// test. Do not "fix" the album titles into something shared without checking
|
||||||
|
// which cap you are then measuring.
|
||||||
|
func seedSharedArtistLibrary(
|
||||||
|
t *testing.T, pool *pgxpool.Pool, name string, numArtists, tracksPerArtist int,
|
||||||
|
) (dbq.User, []pgtype.UUID) {
|
||||||
|
t.Helper()
|
||||||
|
q := dbq.New(pool)
|
||||||
|
ctx := context.Background()
|
||||||
|
u := seedUser(t, pool, name)
|
||||||
|
now := time.Now().UTC()
|
||||||
|
dir := t.TempDir()
|
||||||
|
|
||||||
|
artistIDs := make([]pgtype.UUID, 0, numArtists)
|
||||||
|
for a := 0; a < numArtists; a++ {
|
||||||
|
artistName := name + "-shared-" + string(rune('A'+a))
|
||||||
|
ar, err := q.UpsertArtist(ctx, dbq.UpsertArtistParams{Name: artistName, SortName: artistName})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("seed artist: %v", err)
|
||||||
|
}
|
||||||
|
artistIDs = append(artistIDs, ar.ID)
|
||||||
|
|
||||||
|
for k := 0; k < tracksPerArtist; k++ {
|
||||||
|
albumTitle := fmt.Sprintf("%s - Album %d", artistName, k/2)
|
||||||
|
al, err := q.UpsertAlbum(ctx, dbq.UpsertAlbumParams{
|
||||||
|
Title: albumTitle, SortTitle: albumTitle, ArtistID: ar.ID,
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("seed album: %v", err)
|
||||||
|
}
|
||||||
|
tk, err := q.UpsertTrack(ctx, dbq.UpsertTrackParams{
|
||||||
|
Title: fmt.Sprintf("%s-t%d", artistName, k), AlbumID: al.ID, ArtistID: ar.ID,
|
||||||
|
DurationMs: 1000,
|
||||||
|
FilePath: filepath.Join(dir, fmt.Sprintf("%s-%d-%d.mp3", name, a, k)),
|
||||||
|
FileSize: 100, FileFormat: "mp3",
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("seed track: %v", err)
|
||||||
|
}
|
||||||
|
// Well clear of the recently-played exclusion window, which uses
|
||||||
|
// the DATABASE clock rather than the build's `now`.
|
||||||
|
for pl := 0; pl < 3; pl++ {
|
||||||
|
seedPlayEvent(t, pool, u.ID, tk.ID,
|
||||||
|
now.Add(-time.Duration(24+a*10+k+pl)*time.Hour), false)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return u, artistIDs
|
||||||
|
}
|
||||||
|
|
||||||
|
// The seed artist's own tracks are ELIGIBLE for its "Songs like" mix, and are
|
||||||
|
// bounded by the diversity cap rather than excluded outright (#3881).
|
||||||
|
//
|
||||||
|
// produceSeedMixes used to filter them out — "Songs like X excludes X's own
|
||||||
|
// songs" — which threw away the nearest neighbours of the seed TRACK and then
|
||||||
|
// reached further out to replace them. Operator, 2026-09-10: "it should also
|
||||||
|
// be able to include music from the same artist."
|
||||||
|
//
|
||||||
|
// Asserted end-to-end rather than by reading the source, because the guard has
|
||||||
|
// to survive the filter coming back in a different shape — and because an
|
||||||
|
// absence check would now match the comment explaining why the filter is gone.
|
||||||
|
func TestBuildSystemPlaylists_SongsLikeIncludesItsSeedArtist(t *testing.T) {
|
||||||
|
pool := newPool(t)
|
||||||
|
logger := discardLogger()
|
||||||
|
u, _ := seedSharedArtistLibrary(t, pool, "seedartist", 4, 6)
|
||||||
|
ctx := context.Background()
|
||||||
|
now := time.Date(2026, 5, 4, 12, 0, 0, 0, time.UTC)
|
||||||
|
|
||||||
|
if err := playlists.BuildSystemPlaylists(ctx, pool, logger, u.ID, now, t.TempDir()); err != nil {
|
||||||
|
t.Fatalf("build: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
rows, err := pool.Query(ctx, `
|
||||||
|
SELECT count(*) FILTER (WHERE t.artist_id = p.seed_artist_id) AS own,
|
||||||
|
count(*) AS total
|
||||||
|
FROM playlists p
|
||||||
|
JOIN playlist_tracks pt ON pt.playlist_id = p.id
|
||||||
|
JOIN tracks t ON t.id = pt.track_id
|
||||||
|
WHERE p.user_id = $1 AND p.system_variant = 'songs_like_artist'
|
||||||
|
GROUP BY p.id
|
||||||
|
`, u.ID)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("query: %v", err)
|
||||||
|
}
|
||||||
|
defer rows.Close()
|
||||||
|
|
||||||
|
// discoverMaxTracksPerArtist, which this package_test cannot reference.
|
||||||
|
// Duplicated deliberately: if the cap moves, this failing is the point.
|
||||||
|
const maxPerArtist = 3
|
||||||
|
|
||||||
|
mixes, withOwn := 0, 0
|
||||||
|
for rows.Next() {
|
||||||
|
var own, total int
|
||||||
|
if err := rows.Scan(&own, &total); err != nil {
|
||||||
|
t.Fatalf("scan: %v", err)
|
||||||
|
}
|
||||||
|
mixes++
|
||||||
|
if own > 0 {
|
||||||
|
withOwn++
|
||||||
|
}
|
||||||
|
// The bound is what makes inclusion safe. Without it, "include the
|
||||||
|
// seed artist" becomes "the mix is mostly the seed artist", which is
|
||||||
|
// the radio failure (#3882) arriving on a different surface.
|
||||||
|
if own > maxPerArtist {
|
||||||
|
t.Errorf("a songs_like mix carries %d tracks by its own seed artist "+
|
||||||
|
"out of %d; the per-artist cap (%d) is not being applied",
|
||||||
|
own, total, maxPerArtist)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if err := rows.Err(); err != nil {
|
||||||
|
t.Fatalf("rows: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if mixes == 0 {
|
||||||
|
t.Fatal("no songs_like_artist mixes were built, so this test asserts nothing")
|
||||||
|
}
|
||||||
|
if withOwn == 0 {
|
||||||
|
t.Errorf("none of the %d songs_like mixes contains a single track by its own "+
|
||||||
|
"seed artist — the tracks most likely to sound like the seed are being "+
|
||||||
|
"excluded from the surface whose job is sounding like the seed", mixes)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestBuildSystemPlaylists_DailyNonceDeterminism(t *testing.T) {
|
func TestBuildSystemPlaylists_DailyNonceDeterminism(t *testing.T) {
|
||||||
pool := newPool(t)
|
pool := newPool(t)
|
||||||
logger := discardLogger()
|
logger := discardLogger()
|
||||||
|
|||||||
@@ -105,6 +105,83 @@ func DefaultCandidateSourceLimits() CandidateSourceLimits {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// SongsLikeCandidateSourceLimits is the pool shape for "Songs like {X}"
|
||||||
|
// (#3881). Same total size as the default (~170) — the composition is what
|
||||||
|
// changes, shifted hard toward arms that actually measure distance from the
|
||||||
|
// seed.
|
||||||
|
//
|
||||||
|
// The surface answers "what sounds like THIS", and it shared the default
|
||||||
|
// pool with For-You, which answers the much broader "what will they enjoy
|
||||||
|
// today". Under the default, 50 of ~170 candidates carried sim_score = 0 by
|
||||||
|
// construction — `taste_overlap` (tracks by the user's top taste artists) and
|
||||||
|
// `random_fill` (literally any track not already in the pool), both of which
|
||||||
|
// are seed-INDEPENDENT. Nearly a third of the pool had no relationship to the
|
||||||
|
// seed at all, and the operator saw it: "I was getting a seeming wide variety
|
||||||
|
// of music from each one when I was hoping to stay in a certain neighborhood."
|
||||||
|
//
|
||||||
|
// TIERED, per rule 131 — a system mix degrades, it never vanishes:
|
||||||
|
//
|
||||||
|
// tier 1 lb_similar real track-level similarity. The exact promise.
|
||||||
|
// tier 2 similar_artist, tag_overlap, coplay, likes_overlap
|
||||||
|
// seed-RELATED but weaker signal.
|
||||||
|
// tier 3 taste_overlap, random_fill
|
||||||
|
// seed-independent. The floor, and nothing more.
|
||||||
|
//
|
||||||
|
// The tiering is enforced by SCORE rather than by a fallback ladder: tier-3
|
||||||
|
// arms carry sim_score 0, so under SongsLike weights (SimilarityWeight 4.0,
|
||||||
|
// everything seed-independent demoted) they rank below any real match and
|
||||||
|
// surface only when tiers 1–2 cannot fill the mix. That is the rule's
|
||||||
|
// "fill from tier 1 first, reach down only when a tier cannot fill".
|
||||||
|
//
|
||||||
|
// Which is exactly why tier 3 is REDUCED rather than removed. Zeroing those
|
||||||
|
// two arms was the first instinct and it is the vanish-or-nothing shape rule
|
||||||
|
// 131 exists to forbid: a seed whose artist has thin ListenBrainz coverage
|
||||||
|
// would produce a short mix or none at all, and "no playlist" is a worse
|
||||||
|
// answer than "a few tracks further from the seed than we would like".
|
||||||
|
//
|
||||||
|
// DO NOT SHRINK AN ARM ORDERED BY UNSEEDED random(). This is the constraint
|
||||||
|
// that shapes the numbers below, and it is not obvious from reading them.
|
||||||
|
//
|
||||||
|
// `likes_overlap` and `random_fill` both end in a bare `ORDER BY random()`
|
||||||
|
// (recommendation.sql:118, :161) with no daily seed. Such an arm returns a
|
||||||
|
// STABLE set only while its LIMIT exceeds the rows eligible for it — at that
|
||||||
|
// point it returns all of them and the random order is irrelevant, because
|
||||||
|
// the caller sorts by id before scoring. Drop the limit below the eligible
|
||||||
|
// count and the arm starts returning a random SUBSET, which differs between
|
||||||
|
// two builds on the same day.
|
||||||
|
//
|
||||||
|
// That is a real defect (#3889) rather than a quirk of this function, and it
|
||||||
|
// bit here: cutting RandomFill to 10 broke
|
||||||
|
// TestBuildSystemPlaylists_DailyNonceDeterminism, whose library is smaller
|
||||||
|
// than the default limit and whose determinism was therefore accidental.
|
||||||
|
// Growing an arm is always safe; only shrinking one is.
|
||||||
|
//
|
||||||
|
// So the seed-independent arms are trimmed only where the ordering is
|
||||||
|
// deterministic: `taste_overlap` sorts by `tpa.weight DESC, t.id` and can be
|
||||||
|
// cut, `random_fill` cannot. The reduction is consequently modest — and it
|
||||||
|
// matters less than it looks, because the WEIGHTS are what demote sim_score-0
|
||||||
|
// candidates now. The pool change biases the draw; the songs_like profile is
|
||||||
|
// what actually keeps unrelated tracks out of the result.
|
||||||
|
//
|
||||||
|
// One arm is left alone that arguably should not be: `likes_overlap` assigns
|
||||||
|
// a FLAT 0.6 sim_score (recommendation.sql:108) rather than measuring
|
||||||
|
// anything — a collaborative signal wearing similarity's clothes, which a
|
||||||
|
// raised SimilarityWeight amplifies. If real ListenBrainz scores commonly
|
||||||
|
// land below 0.6 it will outrank genuine matches. It cannot be trimmed here
|
||||||
|
// without the determinism fix landing first; the honest repair is to stop it
|
||||||
|
// claiming a similarity score it never computed (#3879).
|
||||||
|
func SongsLikeCandidateSourceLimits() CandidateSourceLimits {
|
||||||
|
return CandidateSourceLimits{
|
||||||
|
LBSimilar: 60, // tier 1 — doubled; the only arm that measures the seed
|
||||||
|
SimilarArtist: 40, // tier 2 — raised; growing is always safe
|
||||||
|
TagOverlap: 20, // tier 2
|
||||||
|
UserCoplay: 20, // tier 2
|
||||||
|
LikesOverlap: 20, // tier 2 — NOT trimmed: unseeded random(), see above
|
||||||
|
TasteOverlap: 10, // tier 3 floor — halved; deterministic ordering, safe
|
||||||
|
RandomFill: 30, // tier 3 floor — NOT trimmed: unseeded random(), see above
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// LoadCandidatesFromSimilarity is M4c's primary candidate-pool loader.
|
// LoadCandidatesFromSimilarity is M4c's primary candidate-pool loader.
|
||||||
// 5-way SQL UNION (LB-similar / similar-artist tracks / MB-tag overlap /
|
// 5-way SQL UNION (LB-similar / similar-artist tracks / MB-tag overlap /
|
||||||
// likes-overlap / random fill) + dedup-by-max sim_score. Returns
|
// likes-overlap / random fill) + dedup-by-max sim_score. Returns
|
||||||
|
|||||||
@@ -0,0 +1,188 @@
|
|||||||
|
package recommendation
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"math"
|
||||||
|
"sync"
|
||||||
|
"time"
|
||||||
|
|
||||||
|
"github.com/jackc/pgx/v5/pgtype"
|
||||||
|
|
||||||
|
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
||||||
|
)
|
||||||
|
|
||||||
|
// Scaling the candidate pool to the library (#3880).
|
||||||
|
//
|
||||||
|
// DefaultCandidateSourceLimits returns what its own comment calls "the v1
|
||||||
|
// hardcoded constants per spec" — ~170 candidates, identical for a 500-track
|
||||||
|
// library and a 100,000-track one. Operator, 2026-09-10: "is the pool that we
|
||||||
|
// draw from somehow scaled to the amount of music in the library... my earlier
|
||||||
|
// understanding of the tuning and work may have been skewed by what was in my
|
||||||
|
// library."
|
||||||
|
//
|
||||||
|
// It was not, and the consequence compounds. The pool samples a shrinking
|
||||||
|
// FRACTION of the library as it grows — 17% of 1,000 tracks, 1.7% of 10,000,
|
||||||
|
// 0.17% of 100,000 — so the ceiling on how much of a collection can ever
|
||||||
|
// surface stays flat while the collection does not. RandomFill, whose entire
|
||||||
|
// job is exploration, becomes a thinner and noisier slice of a more diverse
|
||||||
|
// corpus at exactly the moment diversity rises.
|
||||||
|
|
||||||
|
const (
|
||||||
|
// libraryScaleReference is the library size at which the base limits
|
||||||
|
// apply unchanged. Below it nothing scales, so small libraries keep
|
||||||
|
// today's behaviour exactly.
|
||||||
|
//
|
||||||
|
// ASSUMED, NOT MEASURED. The size the v1 constants were actually tuned
|
||||||
|
// against is unrecorded; 5,000 is a plausible mid-size library and a
|
||||||
|
// deliberately conservative place to start growing. #3879 should replace
|
||||||
|
// this with the real number, and until it does, this constant is the
|
||||||
|
// single thing to change.
|
||||||
|
libraryScaleReference = 5000
|
||||||
|
|
||||||
|
// maxLibraryScale bounds growth so a very large library does not turn
|
||||||
|
// every recommendation query into a huge scan. At 4x the pool tops out
|
||||||
|
// around 500 candidates, which is still cheap to score in memory.
|
||||||
|
maxLibraryScale = 4.0
|
||||||
|
)
|
||||||
|
|
||||||
|
// libraryScale is sqrt rather than linear on purpose. Linear growth would put
|
||||||
|
// a 100,000-track library at a 3,400-candidate pool — a slow query, slow
|
||||||
|
// scoring, and far past the point where more candidates improve the answer.
|
||||||
|
// Square-root growth keeps the pool meaningfully proportional while staying
|
||||||
|
// bounded: 1x at the reference, 2x at four times it, 4x at sixteen times.
|
||||||
|
//
|
||||||
|
// Never returns below 1: the base limits are a floor, not a midpoint. That
|
||||||
|
// also keeps this safe against #3889 — growing an arm ordered by unseeded
|
||||||
|
// random() is fine, shrinking one is what breaks same-day determinism.
|
||||||
|
func libraryScale(libraryTracks int64) float64 {
|
||||||
|
if libraryTracks <= libraryScaleReference {
|
||||||
|
return 1.0
|
||||||
|
}
|
||||||
|
f := math.Sqrt(float64(libraryTracks) / float64(libraryScaleReference))
|
||||||
|
return math.Min(f, maxLibraryScale)
|
||||||
|
}
|
||||||
|
|
||||||
|
// ScaleForLibrary grows the library-bounded arms of a limit set for a library
|
||||||
|
// of the given size, and leaves the rest alone.
|
||||||
|
//
|
||||||
|
// THE SCALING IS NOT UNIFORM, and that is the substance of it rather than a
|
||||||
|
// refinement. An arm's limit only matters if there are rows for it to cut off,
|
||||||
|
// so what an arm is BOUNDED BY decides whether library size can help it:
|
||||||
|
//
|
||||||
|
// scaled LBSimilar, SimilarArtist, TagOverlap — bounded by similarity
|
||||||
|
// and tag data, which grows as the library does. A bigger library
|
||||||
|
// means more of ListenBrainz's returned MBIDs survive the
|
||||||
|
// local-library filter in similarity/worker.go.
|
||||||
|
// scaled RandomFill — the exploration arm, and the one the complaint is
|
||||||
|
// really about. This is where "fraction of the library" lives.
|
||||||
|
// unscaled LikesOverlap — bounded by the USER's likes.
|
||||||
|
// unscaled UserCoplay — bounded by the INSTANCE's co-play graph.
|
||||||
|
// unscaled TasteOverlap — bounded by the taste profile's artists.
|
||||||
|
//
|
||||||
|
// Raising the last three would not sample more of the library; it would
|
||||||
|
// sample more of a set that did not grow, which is churn rather than reach.
|
||||||
|
// Leaving TasteOverlap alone has a second benefit: it and RandomFill are the
|
||||||
|
// two arms carrying sim_score 0, so this does not inflate the
|
||||||
|
// seed-independent share as fast as a uniform scale would.
|
||||||
|
func ScaleForLibrary(base CandidateSourceLimits, libraryTracks int64) CandidateSourceLimits {
|
||||||
|
f := libraryScale(libraryTracks)
|
||||||
|
grow := func(n int) int { return int(float64(n) * f) } // f >= 1, so never shrinks
|
||||||
|
return CandidateSourceLimits{
|
||||||
|
LBSimilar: grow(base.LBSimilar),
|
||||||
|
SimilarArtist: grow(base.SimilarArtist),
|
||||||
|
TagOverlap: grow(base.TagOverlap),
|
||||||
|
RandomFill: grow(base.RandomFill),
|
||||||
|
LikesOverlap: base.LikesOverlap,
|
||||||
|
UserCoplay: base.UserCoplay,
|
||||||
|
TasteOverlap: base.TasteOverlap,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// CountLibraryTracks returns the whole library's track count.
|
||||||
|
//
|
||||||
|
// Reuses CountTracksMatching with an empty pattern — `title ILIKE '%%'`
|
||||||
|
// matches every row — rather than adding a query, because a new one would
|
||||||
|
// need sqlc regeneration (rule 28, and see #3889 for where that blocks).
|
||||||
|
// The user id is left invalid so the quarantine anti-join is skipped: this
|
||||||
|
// number sizes a pool, and a per-user view of it would be false precision.
|
||||||
|
func CountLibraryTracks(ctx context.Context, q *dbq.Queries) (int64, error) {
|
||||||
|
return q.CountTracksMatching(ctx, dbq.CountTracksMatchingParams{
|
||||||
|
Column1: "",
|
||||||
|
UserID: pgtype.UUID{}, // invalid → NULL → no quarantine filter
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
// librarySizeTTL is how long a counted size is reused. Library size changes
|
||||||
|
// only when a scan runs, so minutes are plenty — and the count is a full
|
||||||
|
// table scan (the ILIKE defeats every index), which is why it is not done
|
||||||
|
// per request.
|
||||||
|
const librarySizeTTL = 5 * time.Minute
|
||||||
|
|
||||||
|
// librarySizeTimeout bounds the count itself — see loadWithDeadline.
|
||||||
|
const librarySizeTimeout = 3 * time.Second
|
||||||
|
|
||||||
|
// LibrarySize memoises the library track count.
|
||||||
|
//
|
||||||
|
// Degrades rather than fails: a count that errors or times out leaves the
|
||||||
|
// previous value in place, and a zero (never yet counted) scales to the base
|
||||||
|
// limits — which is exactly today's behaviour. Nothing about sizing a
|
||||||
|
// candidate pool justifies failing the request it is sizing.
|
||||||
|
type LibrarySize struct {
|
||||||
|
mu sync.Mutex
|
||||||
|
now func() time.Time // injectable for tests
|
||||||
|
at time.Time
|
||||||
|
size int64
|
||||||
|
}
|
||||||
|
|
||||||
|
// NewLibrarySize returns an empty cache. The zero value works too; this
|
||||||
|
// exists so tests can pin the clock.
|
||||||
|
func NewLibrarySize(now func() time.Time) *LibrarySize {
|
||||||
|
return &LibrarySize{now: now}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Get returns the cached size, refreshing through load when stale.
|
||||||
|
//
|
||||||
|
// A NIL RECEIVER IS VALID and means "no cache": the count still runs, it is
|
||||||
|
// just not memoised. That is deliberate rather than defensive habit. The api
|
||||||
|
// handlers struct is built directly by a dozen tests that cannot know about
|
||||||
|
// every field, and a nil here previously panicked inside a radio request —
|
||||||
|
// turning a missing pool-sizing HINT into a 500. Uncached-but-correct is the
|
||||||
|
// right failure for something whose whole contract is that it degrades.
|
||||||
|
func (l *LibrarySize) Get(ctx context.Context, load func(context.Context) (int64, error)) int64 {
|
||||||
|
if l == nil {
|
||||||
|
n, err := loadWithDeadline(ctx, load)
|
||||||
|
if err != nil {
|
||||||
|
return 0 // scales to the base limits
|
||||||
|
}
|
||||||
|
return n
|
||||||
|
}
|
||||||
|
|
||||||
|
l.mu.Lock()
|
||||||
|
defer l.mu.Unlock()
|
||||||
|
|
||||||
|
now := time.Now
|
||||||
|
if l.now != nil {
|
||||||
|
now = l.now
|
||||||
|
}
|
||||||
|
if !l.at.IsZero() && now().Sub(l.at) < librarySizeTTL {
|
||||||
|
return l.size
|
||||||
|
}
|
||||||
|
|
||||||
|
n, err := loadWithDeadline(ctx, load)
|
||||||
|
if err != nil {
|
||||||
|
// Keep the last known value and re-try at the next call rather than
|
||||||
|
// stamping `at`, so a transient failure does not pin a stale number
|
||||||
|
// for the whole TTL.
|
||||||
|
return l.size
|
||||||
|
}
|
||||||
|
l.size, l.at = n, now()
|
||||||
|
return l.size
|
||||||
|
}
|
||||||
|
|
||||||
|
// loadWithDeadline bounds the count. Rule 156: the caller is a user waiting
|
||||||
|
// on a radio, and a pool-sizing hint is never worth hanging for.
|
||||||
|
func loadWithDeadline(ctx context.Context, load func(context.Context) (int64, error)) (int64, error) {
|
||||||
|
cctx, cancel := context.WithTimeout(ctx, librarySizeTimeout)
|
||||||
|
defer cancel()
|
||||||
|
return load(cctx)
|
||||||
|
}
|
||||||
@@ -0,0 +1,216 @@
|
|||||||
|
package recommendation
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"errors"
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
)
|
||||||
|
|
||||||
|
// Small libraries must behave exactly as before. Scaling is meant to help a
|
||||||
|
// growing collection, not to change what every existing install already does.
|
||||||
|
func TestScaleForLibrary_SmallLibrariesAreUntouched(t *testing.T) {
|
||||||
|
base := DefaultCandidateSourceLimits()
|
||||||
|
for _, size := range []int64{0, 1, 500, libraryScaleReference} {
|
||||||
|
if got := ScaleForLibrary(base, size); got != base {
|
||||||
|
t.Errorf("library of %d changed the limits: %+v, want %+v", size, got, base)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// The complaint, restated as a property: a bigger library must reach further
|
||||||
|
// into itself. Operator, 2026-09-10 — "is the pool that we draw from somehow
|
||||||
|
// scaled to the amount of music in the library".
|
||||||
|
func TestScaleForLibrary_BigLibrariesGetABiggerPool(t *testing.T) {
|
||||||
|
base := DefaultCandidateSourceLimits()
|
||||||
|
small := ScaleForLibrary(base, libraryScaleReference)
|
||||||
|
big := ScaleForLibrary(base, libraryScaleReference*16)
|
||||||
|
|
||||||
|
if big.RandomFill <= small.RandomFill {
|
||||||
|
t.Errorf("RandomFill %d did not grow for a 16x library (was %d); that arm "+
|
||||||
|
"IS the fraction-of-library problem", big.RandomFill, small.RandomFill)
|
||||||
|
}
|
||||||
|
if big.LBSimilar <= small.LBSimilar {
|
||||||
|
t.Errorf("LBSimilar %d did not grow for a 16x library (was %d)",
|
||||||
|
big.LBSimilar, small.LBSimilar)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// THE SUBSTANCE: scaling is per-arm, decided by what each arm is BOUNDED BY.
|
||||||
|
// Raising a limit only helps if there are rows for it to cut off, so arms
|
||||||
|
// bounded by the user's likes, the instance's co-play graph or the taste
|
||||||
|
// profile gain nothing from a bigger library — raising them would sample more
|
||||||
|
// of a set that did not grow.
|
||||||
|
//
|
||||||
|
// A uniform scale would look right and be wrong, which is why this is pinned
|
||||||
|
// separately from "the pool got bigger".
|
||||||
|
func TestScaleForLibrary_OnlyLibraryBoundedArmsGrow(t *testing.T) {
|
||||||
|
base := DefaultCandidateSourceLimits()
|
||||||
|
big := ScaleForLibrary(base, libraryScaleReference*16)
|
||||||
|
|
||||||
|
for _, tc := range []struct {
|
||||||
|
arm string
|
||||||
|
got, want int
|
||||||
|
why string
|
||||||
|
}{
|
||||||
|
{"LikesOverlap", big.LikesOverlap, base.LikesOverlap, "bounded by the user's likes"},
|
||||||
|
{"UserCoplay", big.UserCoplay, base.UserCoplay, "bounded by the instance's co-play graph"},
|
||||||
|
{"TasteOverlap", big.TasteOverlap, base.TasteOverlap, "bounded by the taste profile's artists"},
|
||||||
|
} {
|
||||||
|
if tc.got != tc.want {
|
||||||
|
t.Errorf("%s scaled to %d (base %d) but is %s — a bigger library gives it "+
|
||||||
|
"nothing more to return", tc.arm, tc.got, tc.want, tc.why)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Growth is bounded, or a huge library turns every recommendation query into
|
||||||
|
// a huge scan. sqrt keeps it proportional; the ceiling keeps it affordable.
|
||||||
|
func TestScaleForLibrary_GrowthIsBounded(t *testing.T) {
|
||||||
|
base := DefaultCandidateSourceLimits()
|
||||||
|
huge := ScaleForLibrary(base, 100_000_000)
|
||||||
|
|
||||||
|
if huge.RandomFill > int(float64(base.RandomFill)*maxLibraryScale) {
|
||||||
|
t.Errorf("RandomFill %d exceeds the %.0fx ceiling on base %d",
|
||||||
|
huge.RandomFill, maxLibraryScale, base.RandomFill)
|
||||||
|
}
|
||||||
|
// sqrt, not linear — and the test point matters. At SIXTEEN times the
|
||||||
|
// reference both curves land on 4x, because linear has already been
|
||||||
|
// clamped by the ceiling; asserting there proves nothing. Four times the
|
||||||
|
// reference is below the ceiling for both, so the curves separate: sqrt
|
||||||
|
// gives 2x, linear would give 4x.
|
||||||
|
four := ScaleForLibrary(base, libraryScaleReference*4)
|
||||||
|
if four.RandomFill != base.RandomFill*2 {
|
||||||
|
t.Errorf("4x library gave RandomFill %d, want %d — sqrt growth (linear "+
|
||||||
|
"would give %d)", four.RandomFill, base.RandomFill*2, base.RandomFill*4)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Never below the base. The base limits are a floor, not a midpoint — and
|
||||||
|
// #3889 makes this load-bearing rather than tidy: shrinking an arm ordered by
|
||||||
|
// unseeded random() changes pool membership between same-day rebuilds.
|
||||||
|
func TestScaleForLibrary_NeverShrinksAnArm(t *testing.T) {
|
||||||
|
base := DefaultCandidateSourceLimits()
|
||||||
|
for _, size := range []int64{0, 1, 100, 4999, 5001, 1_000_000} {
|
||||||
|
got := ScaleForLibrary(base, size)
|
||||||
|
for _, tc := range []struct {
|
||||||
|
arm string
|
||||||
|
got, want int
|
||||||
|
}{
|
||||||
|
{"LBSimilar", got.LBSimilar, base.LBSimilar},
|
||||||
|
{"SimilarArtist", got.SimilarArtist, base.SimilarArtist},
|
||||||
|
{"TagOverlap", got.TagOverlap, base.TagOverlap},
|
||||||
|
{"RandomFill", got.RandomFill, base.RandomFill},
|
||||||
|
{"LikesOverlap", got.LikesOverlap, base.LikesOverlap},
|
||||||
|
{"UserCoplay", got.UserCoplay, base.UserCoplay},
|
||||||
|
{"TasteOverlap", got.TasteOverlap, base.TasteOverlap},
|
||||||
|
} {
|
||||||
|
if tc.got < tc.want {
|
||||||
|
t.Errorf("library %d shrank %s to %d (base %d)", size, tc.arm, tc.got, tc.want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// A pool-sizing hint must never fail the request it is sizing. A count that
|
||||||
|
// errors leaves the previous value in place, and a never-counted cache
|
||||||
|
// returns 0 — which scales to the base limits, i.e. exactly today's
|
||||||
|
// behaviour.
|
||||||
|
func TestLibrarySize_DegradesOnFailure(t *testing.T) {
|
||||||
|
clock := time.Now()
|
||||||
|
c := NewLibrarySize(func() time.Time { return clock })
|
||||||
|
|
||||||
|
boom := func(context.Context) (int64, error) { return 0, errors.New("db is down") }
|
||||||
|
if got := c.Get(context.Background(), boom); got != 0 {
|
||||||
|
t.Errorf("first failure returned %d, want 0 (which scales to the base limits)", got)
|
||||||
|
}
|
||||||
|
if got := ScaleForLibrary(DefaultCandidateSourceLimits(), 0); got != DefaultCandidateSourceLimits() {
|
||||||
|
t.Error("a zero library size did not scale to the base limits")
|
||||||
|
}
|
||||||
|
|
||||||
|
// A good count, then a failure: the last known value survives.
|
||||||
|
if got := c.Get(context.Background(), func(context.Context) (int64, error) { return 40_000, nil }); got != 40_000 {
|
||||||
|
t.Fatalf("got %d, want 40000", got)
|
||||||
|
}
|
||||||
|
clock = clock.Add(librarySizeTTL + time.Second)
|
||||||
|
if got := c.Get(context.Background(), boom); got != 40_000 {
|
||||||
|
t.Errorf("a failed refresh returned %d, discarding the last known 40000", got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// The count is a full table scan, so it must not run per request.
|
||||||
|
func TestLibrarySize_CachesWithinTheTTL(t *testing.T) {
|
||||||
|
clock := time.Now()
|
||||||
|
c := NewLibrarySize(func() time.Time { return clock })
|
||||||
|
calls := 0
|
||||||
|
load := func(context.Context) (int64, error) { calls++; return 1234, nil }
|
||||||
|
|
||||||
|
for i := 0; i < 5; i++ {
|
||||||
|
c.Get(context.Background(), load)
|
||||||
|
}
|
||||||
|
if calls != 1 {
|
||||||
|
t.Errorf("counted %d times within the TTL, want 1 — this is a full table scan", calls)
|
||||||
|
}
|
||||||
|
|
||||||
|
clock = clock.Add(librarySizeTTL + time.Second)
|
||||||
|
c.Get(context.Background(), load)
|
||||||
|
if calls != 2 {
|
||||||
|
t.Errorf("counted %d times after the TTL expired, want 2 — the size never refreshes", calls)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// A transient failure must not pin a stale value for the whole TTL: the
|
||||||
|
// failed refresh does not stamp the clock, so the next call retries.
|
||||||
|
func TestLibrarySize_RetriesAfterAFailedRefresh(t *testing.T) {
|
||||||
|
clock := time.Now()
|
||||||
|
c := NewLibrarySize(func() time.Time { return clock })
|
||||||
|
|
||||||
|
c.Get(context.Background(), func(context.Context) (int64, error) { return 100, nil })
|
||||||
|
clock = clock.Add(librarySizeTTL + time.Second)
|
||||||
|
c.Get(context.Background(), func(context.Context) (int64, error) { return 0, errors.New("blip") })
|
||||||
|
|
||||||
|
// Immediately after, with no clock movement — a stamped failure would
|
||||||
|
// serve the stale 100 until the TTL expired again.
|
||||||
|
if got := c.Get(context.Background(), func(context.Context) (int64, error) { return 900, nil }); got != 900 {
|
||||||
|
t.Errorf("got %d after a failed refresh, want 900 — the failure pinned a stale value", got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// A nil cache must not panic, and must still be CORRECT — uncached, not
|
||||||
|
// broken.
|
||||||
|
//
|
||||||
|
// This is not a hypothetical hardening. internal/api builds its handlers
|
||||||
|
// struct directly in a dozen tests, none of which know about every field, so
|
||||||
|
// librarySize arrives nil there. The first version of this panicked inside
|
||||||
|
// handleRadio and took down TestHandleRadio_ColdStart_OnlySeedReturned with a
|
||||||
|
// SIGSEGV — turning a missing pool-sizing HINT into a request-killing crash,
|
||||||
|
// which is the opposite of what a value that "degrades rather than fails" is
|
||||||
|
// supposed to do.
|
||||||
|
func TestLibrarySize_NilReceiverStillCounts(t *testing.T) {
|
||||||
|
var c *LibrarySize // deliberately not constructed
|
||||||
|
|
||||||
|
calls := 0
|
||||||
|
got := c.Get(context.Background(), func(context.Context) (int64, error) {
|
||||||
|
calls++
|
||||||
|
return 40_000, nil
|
||||||
|
})
|
||||||
|
if got != 40_000 {
|
||||||
|
t.Errorf("nil cache returned %d, want 40000 — it should still count, just not memoise", got)
|
||||||
|
}
|
||||||
|
if calls != 1 {
|
||||||
|
t.Errorf("nil cache called the loader %d times, want 1", calls)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Uncached: a second call counts again rather than reusing anything.
|
||||||
|
c.Get(context.Background(), func(context.Context) (int64, error) { calls++; return 40_000, nil })
|
||||||
|
if calls != 2 {
|
||||||
|
t.Errorf("nil cache memoised across calls (%d loads); it has nowhere to store a value", calls)
|
||||||
|
}
|
||||||
|
|
||||||
|
// And it still degrades on error rather than panicking.
|
||||||
|
if got := c.Get(context.Background(), func(context.Context) (int64, error) {
|
||||||
|
return 0, errors.New("db is down")
|
||||||
|
}); got != 0 {
|
||||||
|
t.Errorf("nil cache returned %d on a failed count, want 0 (base limits)", got)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -4,6 +4,8 @@ import (
|
|||||||
"sort"
|
"sort"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
|
"github.com/jackc/pgx/v5/pgtype"
|
||||||
|
|
||||||
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -13,15 +15,72 @@ type Candidate struct {
|
|||||||
Inputs ScoringInputs
|
Inputs ScoringInputs
|
||||||
}
|
}
|
||||||
|
|
||||||
// Shuffle scores each candidate, sorts descending by score, and returns
|
// DiversityCaps bounds how much of one selection a single artist or album
|
||||||
// the top `limit` candidates. limit <= 0 returns nil; nil input returns
|
// may occupy. A zero value means "no cap", which is what Shuffle did
|
||||||
// nil. Pure — no IO, no global state beyond the rng callback.
|
// unconditionally before #3882.
|
||||||
|
type DiversityCaps struct {
|
||||||
|
MaxPerArtist int
|
||||||
|
MaxPerAlbum int
|
||||||
|
}
|
||||||
|
|
||||||
|
// radioCapArtistPer25 / radioCapAlbumPer25 hold the caps at the same
|
||||||
|
// PROPORTION the system mixes already use — 3 artist / 2 album tracks in a
|
||||||
|
// 25-track mix — so a 20-track radio and a 200-track one feel alike rather
|
||||||
|
// than one of them being effectively uncapped.
|
||||||
|
//
|
||||||
|
// A fixed count cannot do that. Three-per-artist is a reasonable 12% of a
|
||||||
|
// 25-track mix and an absurd 1.5% of a 200-track radio, where it would put
|
||||||
|
// the selection permanently in the relaxation path below and quietly undo
|
||||||
|
// the cap it was meant to enforce.
|
||||||
|
const (
|
||||||
|
radioCapArtistPer25 = 3
|
||||||
|
radioCapAlbumPer25 = 2
|
||||||
|
)
|
||||||
|
|
||||||
|
// RadioDiversityCaps scales the diversity caps to the requested radio
|
||||||
|
// length. Floors of 2 and 1 keep a very short radio from being capped into
|
||||||
|
// a single track per artist, which would be its own kind of wrong.
|
||||||
|
func RadioDiversityCaps(limit int) DiversityCaps {
|
||||||
|
artist := limit * radioCapArtistPer25 / 25
|
||||||
|
if artist < 2 {
|
||||||
|
artist = 2
|
||||||
|
}
|
||||||
|
album := limit * radioCapAlbumPer25 / 25
|
||||||
|
if album < 1 {
|
||||||
|
album = 1
|
||||||
|
}
|
||||||
|
return DiversityCaps{MaxPerArtist: artist, MaxPerAlbum: album}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Shuffle scores each candidate, sorts descending by score, and returns the
|
||||||
|
// top `limit` candidates, preferring artist/album diversity. limit <= 0
|
||||||
|
// returns nil; nil input returns nil. Pure — no IO, no global state beyond
|
||||||
|
// the rng callback.
|
||||||
|
//
|
||||||
|
// THE COUNT IS NEVER REDUCED BY THE CAPS. Selection runs in two passes: the
|
||||||
|
// first takes candidates that fit under the caps, and the second fills any
|
||||||
|
// remaining slots from those the first pass skipped, still in score order.
|
||||||
|
// So the result holds min(limit, len(candidates)) either way — the caps
|
||||||
|
// change WHICH tracks are chosen, never HOW MANY.
|
||||||
|
//
|
||||||
|
// That two-pass shape is the whole design, and a hard cap would have been
|
||||||
|
// the easy mistake. Radio asks for 50 tracks by default and 200 at most; a
|
||||||
|
// pool concentrated on a few artists would return six tracks and call it a
|
||||||
|
// radio. Rule 131's principle — degrade, never vanish — applies past the
|
||||||
|
// system mixes it was written for.
|
||||||
|
//
|
||||||
|
// Before #3882 there was no cap here at all, while discover.go,
|
||||||
|
// you_might_like.go and home.go all had one. That asymmetry is what let a
|
||||||
|
// radio session come back entirely from a single artist: nothing between
|
||||||
|
// the pool and the output bounded any artist's share, so a pool dominated
|
||||||
|
// by one artist produced an output dominated by it too.
|
||||||
func Shuffle(
|
func Shuffle(
|
||||||
candidates []Candidate,
|
candidates []Candidate,
|
||||||
weights ScoringWeights,
|
weights ScoringWeights,
|
||||||
now time.Time,
|
now time.Time,
|
||||||
rng func() float64,
|
rng func() float64,
|
||||||
limit int,
|
limit int,
|
||||||
|
caps DiversityCaps,
|
||||||
) []Candidate {
|
) []Candidate {
|
||||||
if len(candidates) == 0 || limit <= 0 {
|
if len(candidates) == 0 || limit <= 0 {
|
||||||
return nil
|
return nil
|
||||||
@@ -40,9 +99,36 @@ func Shuffle(
|
|||||||
if limit > len(scored) {
|
if limit > len(scored) {
|
||||||
limit = len(scored)
|
limit = len(scored)
|
||||||
}
|
}
|
||||||
out := make([]Candidate, limit)
|
|
||||||
for i := 0; i < limit; i++ {
|
out := make([]Candidate, 0, limit)
|
||||||
out[i] = scored[i].c
|
deferred := make([]Candidate, 0, len(scored)-limit)
|
||||||
|
artistCount := map[pgtype.UUID]int{}
|
||||||
|
albumCount := map[pgtype.UUID]int{}
|
||||||
|
|
||||||
|
for _, s := range scored {
|
||||||
|
if len(out) == limit {
|
||||||
|
break
|
||||||
|
}
|
||||||
|
overArtist := caps.MaxPerArtist > 0 && artistCount[s.c.Track.ArtistID] >= caps.MaxPerArtist
|
||||||
|
overAlbum := caps.MaxPerAlbum > 0 && albumCount[s.c.Track.AlbumID] >= caps.MaxPerAlbum
|
||||||
|
if overArtist || overAlbum {
|
||||||
|
// Held back, not discarded — pass two may still need it.
|
||||||
|
deferred = append(deferred, s.c)
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
artistCount[s.c.Track.ArtistID]++
|
||||||
|
albumCount[s.c.Track.AlbumID]++
|
||||||
|
out = append(out, s.c)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Pass two: the caps could not fill the request, so relax them rather
|
||||||
|
// than hand back a short radio. Still score order, so the best of the
|
||||||
|
// held-back candidates go first.
|
||||||
|
for _, c := range deferred {
|
||||||
|
if len(out) == limit {
|
||||||
|
break
|
||||||
|
}
|
||||||
|
out = append(out, c)
|
||||||
}
|
}
|
||||||
return out
|
return out
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,197 @@
|
|||||||
|
package recommendation
|
||||||
|
|
||||||
|
import (
|
||||||
|
"fmt"
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
|
"github.com/jackc/pgx/v5/pgtype"
|
||||||
|
|
||||||
|
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
||||||
|
)
|
||||||
|
|
||||||
|
// candBy builds a candidate with a real artist and album identity, which
|
||||||
|
// `cand` deliberately leaves zero.
|
||||||
|
func candBy(t *testing.T, id, artist, album string, in ScoringInputs) Candidate {
|
||||||
|
t.Helper()
|
||||||
|
tr := dbq.Track{Title: id}
|
||||||
|
if err := tr.ID.Scan("00000000-0000-0000-0000-" + id); err != nil {
|
||||||
|
t.Fatalf("track id %q: %v", id, err)
|
||||||
|
}
|
||||||
|
if err := tr.ArtistID.Scan("00000000-0000-0000-0001-" + artist); err != nil {
|
||||||
|
t.Fatalf("artist id %q: %v", artist, err)
|
||||||
|
}
|
||||||
|
if err := tr.AlbumID.Scan("00000000-0000-0000-0002-" + album); err != nil {
|
||||||
|
t.Fatalf("album id %q: %v", album, err)
|
||||||
|
}
|
||||||
|
return Candidate{Track: tr, Inputs: in}
|
||||||
|
}
|
||||||
|
|
||||||
|
// artistKey is the map key artistsOf produces for a given fixture artist,
|
||||||
|
// derived the same way the fixture builds the UUID. Hand-writing the hex is
|
||||||
|
// how the first version of this test went wrong: the artist id is not all
|
||||||
|
// zeros — it carries 0001 in its fourth group — so the literal did not match
|
||||||
|
// and the assertion measured nothing.
|
||||||
|
func artistKey(t *testing.T, artist string) string {
|
||||||
|
t.Helper()
|
||||||
|
var id pgtype.UUID
|
||||||
|
if err := id.Scan("00000000-0000-0000-0001-" + artist); err != nil {
|
||||||
|
t.Fatalf("artist id %q: %v", artist, err)
|
||||||
|
}
|
||||||
|
return fmt.Sprintf("%x", id.Bytes)
|
||||||
|
}
|
||||||
|
|
||||||
|
// artistsOf counts how many picks each artist contributed.
|
||||||
|
func artistsOf(picks []Candidate) map[string]int {
|
||||||
|
out := map[string]int{}
|
||||||
|
for _, p := range picks {
|
||||||
|
out[fmt.Sprintf("%x", p.Track.ArtistID.Bytes)]++
|
||||||
|
}
|
||||||
|
return out
|
||||||
|
}
|
||||||
|
|
||||||
|
// THE BUG. Operator, 2026-09-10: started radio from a song and "literally
|
||||||
|
// all of the songs in the playlist after that were from a single artist".
|
||||||
|
//
|
||||||
|
// One artist's tracks all outscore everything else, and there are more of
|
||||||
|
// them than the radio has room for. Without a cap the output is entirely
|
||||||
|
// that artist — nothing between the pool and the result bounded its share.
|
||||||
|
func TestShuffle_CapStopsOneArtistTakingTheWholeRadio(t *testing.T) {
|
||||||
|
var cs []Candidate
|
||||||
|
// 20 tracks by artist A, all liked so they sort to the top.
|
||||||
|
for i := 0; i < 20; i++ {
|
||||||
|
cs = append(cs, candBy(t, fmt.Sprintf("%012d", i), "00000000000a",
|
||||||
|
fmt.Sprintf("%012d", i), ScoringInputs{IsGeneralLiked: true}))
|
||||||
|
}
|
||||||
|
// 10 tracks by 10 other artists, none liked, so they all rank below.
|
||||||
|
for i := 0; i < 10; i++ {
|
||||||
|
cs = append(cs, candBy(t, fmt.Sprintf("%012d", 100+i),
|
||||||
|
fmt.Sprintf("%012d", 200+i), fmt.Sprintf("%012d", 100+i),
|
||||||
|
ScoringInputs{IsGeneralLiked: false}))
|
||||||
|
}
|
||||||
|
|
||||||
|
const limit = 10
|
||||||
|
caps := DiversityCaps{MaxPerArtist: 3}
|
||||||
|
picks := Shuffle(cs, defaultWeights(), time.Now(), fixedRNG(0.5), limit, caps)
|
||||||
|
|
||||||
|
if len(picks) != limit {
|
||||||
|
t.Fatalf("len = %d, want %d", len(picks), limit)
|
||||||
|
}
|
||||||
|
byArtist := artistsOf(picks)
|
||||||
|
artistA := artistKey(t, "00000000000a")
|
||||||
|
if got := byArtist[artistA]; got != 3 {
|
||||||
|
t.Errorf("artist A contributed %d of %d picks, cap is 3", got, limit)
|
||||||
|
}
|
||||||
|
if len(byArtist) < 8 {
|
||||||
|
t.Errorf("only %d distinct artists in a %d-track radio; the cap is not "+
|
||||||
|
"spreading the selection", len(byArtist), limit)
|
||||||
|
}
|
||||||
|
|
||||||
|
// The same pool with NO cap is the regression this exists to catch. If
|
||||||
|
// this stops holding, the test above is no longer proving anything.
|
||||||
|
uncapped := Shuffle(cs, defaultWeights(), time.Now(), fixedRNG(0.5), limit, DiversityCaps{})
|
||||||
|
if artistsOf(uncapped)[artistA] != limit {
|
||||||
|
t.Errorf("uncapped selection was not single-artist, so this fixture no " +
|
||||||
|
"longer reproduces the bug being fixed")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// A CAP IS A BOUND, NOT AN EXCLUSION. The operator asked for the opposite of
|
||||||
|
// removal: "again it should be able to add songs from the same artist."
|
||||||
|
func TestShuffle_CappedArtistIsStillRepresented(t *testing.T) {
|
||||||
|
var cs []Candidate
|
||||||
|
for i := 0; i < 20; i++ {
|
||||||
|
cs = append(cs, candBy(t, fmt.Sprintf("%012d", i), "00000000000a",
|
||||||
|
fmt.Sprintf("%012d", i), ScoringInputs{IsGeneralLiked: true}))
|
||||||
|
}
|
||||||
|
for i := 0; i < 10; i++ {
|
||||||
|
cs = append(cs, candBy(t, fmt.Sprintf("%012d", 100+i),
|
||||||
|
fmt.Sprintf("%012d", 200+i), fmt.Sprintf("%012d", 100+i),
|
||||||
|
ScoringInputs{IsGeneralLiked: false}))
|
||||||
|
}
|
||||||
|
picks := Shuffle(cs, defaultWeights(), time.Now(), fixedRNG(0.5), 10,
|
||||||
|
DiversityCaps{MaxPerArtist: 3})
|
||||||
|
if artistsOf(picks)[artistKey(t, "00000000000a")] == 0 {
|
||||||
|
t.Error("the dominant artist was excluded entirely; the cap should bound " +
|
||||||
|
"its share, not remove it")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// RULE 131, applied past the system mixes it was written for: degrade, never
|
||||||
|
// vanish. A hard cap over a pool with few artists would hand back a six-track
|
||||||
|
// "radio" for a fifty-track request. The caps must change WHICH tracks are
|
||||||
|
// picked, never HOW MANY.
|
||||||
|
func TestShuffle_CapsNeverShortenTheResult(t *testing.T) {
|
||||||
|
for _, tc := range []struct {
|
||||||
|
name string
|
||||||
|
artists int
|
||||||
|
perArt int
|
||||||
|
limit int
|
||||||
|
}{
|
||||||
|
{"one artist owns the entire pool", 1, 30, 10},
|
||||||
|
{"two artists, tight cap", 2, 15, 20},
|
||||||
|
{"pool smaller than the request", 3, 2, 50},
|
||||||
|
} {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
var cs []Candidate
|
||||||
|
n := 0
|
||||||
|
for a := 0; a < tc.artists; a++ {
|
||||||
|
for k := 0; k < tc.perArt; k++ {
|
||||||
|
cs = append(cs, candBy(t, fmt.Sprintf("%012d", n),
|
||||||
|
fmt.Sprintf("%012d", 300+a), fmt.Sprintf("%012d", n),
|
||||||
|
ScoringInputs{}))
|
||||||
|
n++
|
||||||
|
}
|
||||||
|
}
|
||||||
|
want := tc.limit
|
||||||
|
if len(cs) < want {
|
||||||
|
want = len(cs)
|
||||||
|
}
|
||||||
|
picks := Shuffle(cs, defaultWeights(), time.Now(), fixedRNG(0.5), tc.limit,
|
||||||
|
DiversityCaps{MaxPerArtist: 3, MaxPerAlbum: 2})
|
||||||
|
if len(picks) != want {
|
||||||
|
t.Errorf("len = %d, want %d — the caps shortened the result instead "+
|
||||||
|
"of relaxing to fill it", len(picks), want)
|
||||||
|
}
|
||||||
|
// No duplicates: a candidate deferred in pass one must not also be
|
||||||
|
// taken in pass two.
|
||||||
|
seen := map[[16]byte]bool{}
|
||||||
|
for _, p := range picks {
|
||||||
|
if seen[p.Track.ID.Bytes] {
|
||||||
|
t.Fatalf("track %x appears twice; pass two re-added a pick", p.Track.ID.Bytes)
|
||||||
|
}
|
||||||
|
seen[p.Track.ID.Bytes] = true
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// A fixed count cannot serve both a 20-track radio and a 200-track one: three
|
||||||
|
// per artist is 12% of the first and 1.5% of the second, which would leave the
|
||||||
|
// long radio permanently in the relaxation path and effectively uncapped.
|
||||||
|
func TestRadioDiversityCaps_ScaleWithTheRequestedLength(t *testing.T) {
|
||||||
|
short := RadioDiversityCaps(25)
|
||||||
|
long := RadioDiversityCaps(200)
|
||||||
|
|
||||||
|
if short.MaxPerArtist != 3 {
|
||||||
|
t.Errorf("a 25-track radio caps artists at %d, want 3 — the same "+
|
||||||
|
"proportion the system mixes use", short.MaxPerArtist)
|
||||||
|
}
|
||||||
|
if long.MaxPerArtist <= short.MaxPerArtist {
|
||||||
|
t.Errorf("a 200-track radio caps artists at %d, no higher than a 25-track "+
|
||||||
|
"one at %d; the cap is not scaling", long.MaxPerArtist, short.MaxPerArtist)
|
||||||
|
}
|
||||||
|
// Proportion held, not just "bigger".
|
||||||
|
if long.MaxPerArtist != 24 {
|
||||||
|
t.Errorf("200-track artist cap = %d, want 24 (3 per 25)", long.MaxPerArtist)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Floors: a tiny radio must not be capped down to one track per artist.
|
||||||
|
tiny := RadioDiversityCaps(1)
|
||||||
|
if tiny.MaxPerArtist < 2 {
|
||||||
|
t.Errorf("tiny radio artist cap = %d, want at least 2", tiny.MaxPerArtist)
|
||||||
|
}
|
||||||
|
if tiny.MaxPerAlbum < 1 {
|
||||||
|
t.Errorf("tiny radio album cap = %d, want at least 1", tiny.MaxPerAlbum)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -21,7 +21,7 @@ func TestShuffle_LikedRanksAboveUnliked(t *testing.T) {
|
|||||||
cand("000000000001", ScoringInputs{IsGeneralLiked: false}),
|
cand("000000000001", ScoringInputs{IsGeneralLiked: false}),
|
||||||
cand("000000000002", ScoringInputs{IsGeneralLiked: true}),
|
cand("000000000002", ScoringInputs{IsGeneralLiked: true}),
|
||||||
}
|
}
|
||||||
out := Shuffle(cs, defaultWeights(), time.Now(), fixedRNG(0.5), 10)
|
out := Shuffle(cs, defaultWeights(), time.Now(), fixedRNG(0.5), 10, DiversityCaps{})
|
||||||
if out[0].Track.Title != "000000000002" {
|
if out[0].Track.Title != "000000000002" {
|
||||||
t.Errorf("liked track did not rank first: %+v", out)
|
t.Errorf("liked track did not rank first: %+v", out)
|
||||||
}
|
}
|
||||||
@@ -33,7 +33,7 @@ func TestShuffle_HighSkipRanksLast(t *testing.T) {
|
|||||||
cand("000000000002", ScoringInputs{PlayCount: 10, SkipCount: 0}), // ratio 0
|
cand("000000000002", ScoringInputs{PlayCount: 10, SkipCount: 0}), // ratio 0
|
||||||
cand("000000000003", ScoringInputs{PlayCount: 10, SkipCount: 5}), // ratio 0.5
|
cand("000000000003", ScoringInputs{PlayCount: 10, SkipCount: 5}), // ratio 0.5
|
||||||
}
|
}
|
||||||
out := Shuffle(cs, defaultWeights(), time.Now(), fixedRNG(0.5), 10)
|
out := Shuffle(cs, defaultWeights(), time.Now(), fixedRNG(0.5), 10, DiversityCaps{})
|
||||||
if out[0].Track.Title != "000000000002" || out[2].Track.Title != "000000000001" {
|
if out[0].Track.Title != "000000000002" || out[2].Track.Title != "000000000001" {
|
||||||
t.Errorf("skip-ratio ordering broken: %v", titles(out))
|
t.Errorf("skip-ratio ordering broken: %v", titles(out))
|
||||||
}
|
}
|
||||||
@@ -44,7 +44,7 @@ func TestShuffle_LimitTruncates(t *testing.T) {
|
|||||||
for i := range cs {
|
for i := range cs {
|
||||||
cs[i] = cand("00000000000"+string(rune('a'+i%26)), ScoringInputs{})
|
cs[i] = cand("00000000000"+string(rune('a'+i%26)), ScoringInputs{})
|
||||||
}
|
}
|
||||||
out := Shuffle(cs, defaultWeights(), time.Now(), fixedRNG(0.5), 10)
|
out := Shuffle(cs, defaultWeights(), time.Now(), fixedRNG(0.5), 10, DiversityCaps{})
|
||||||
if len(out) != 10 {
|
if len(out) != 10 {
|
||||||
t.Errorf("len = %d, want 10", len(out))
|
t.Errorf("len = %d, want 10", len(out))
|
||||||
}
|
}
|
||||||
@@ -58,7 +58,7 @@ func TestShuffle_JitterDoesNotFlipStructuralWinner(t *testing.T) {
|
|||||||
cand("000000000001", ScoringInputs{IsGeneralLiked: false}),
|
cand("000000000001", ScoringInputs{IsGeneralLiked: false}),
|
||||||
cand("000000000002", ScoringInputs{IsGeneralLiked: true}),
|
cand("000000000002", ScoringInputs{IsGeneralLiked: true}),
|
||||||
}
|
}
|
||||||
out := Shuffle(cs, defaultWeights(), time.Now(), r.Float64, 10)
|
out := Shuffle(cs, defaultWeights(), time.Now(), r.Float64, 10, DiversityCaps{})
|
||||||
if out[0].Track.Title != "000000000002" {
|
if out[0].Track.Title != "000000000002" {
|
||||||
t.Fatalf("iter %d: liked did not rank first; out=%v", i, titles(out))
|
t.Fatalf("iter %d: liked did not rank first; out=%v", i, titles(out))
|
||||||
}
|
}
|
||||||
@@ -66,7 +66,7 @@ func TestShuffle_JitterDoesNotFlipStructuralWinner(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func TestShuffle_Empty_ReturnsEmpty(t *testing.T) {
|
func TestShuffle_Empty_ReturnsEmpty(t *testing.T) {
|
||||||
out := Shuffle(nil, defaultWeights(), time.Now(), fixedRNG(0.5), 10)
|
out := Shuffle(nil, defaultWeights(), time.Now(), fixedRNG(0.5), 10, DiversityCaps{})
|
||||||
if len(out) != 0 {
|
if len(out) != 0 {
|
||||||
t.Errorf("len = %d, want 0", len(out))
|
t.Errorf("len = %d, want 0", len(out))
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,99 @@
|
|||||||
|
package recommendation
|
||||||
|
|
||||||
|
import "testing"
|
||||||
|
|
||||||
|
// Songs-like's pool must lean on arms that MEASURE distance from the seed.
|
||||||
|
// The default gave ~29% of candidates a sim_score of literally 0
|
||||||
|
// (taste_overlap and random_fill are both `0.0::float8` in
|
||||||
|
// recommendation.sql), which is what let "Songs like X" wander.
|
||||||
|
func TestSongsLikeLimits_FavourTheArmsThatMeasureTheSeed(t *testing.T) {
|
||||||
|
d := DefaultCandidateSourceLimits()
|
||||||
|
s := SongsLikeCandidateSourceLimits()
|
||||||
|
|
||||||
|
if s.LBSimilar <= d.LBSimilar {
|
||||||
|
t.Errorf("LBSimilar %d is not above the default %d — the only arm that "+
|
||||||
|
"measures track-level distance from the seed should be favoured here",
|
||||||
|
s.LBSimilar, d.LBSimilar)
|
||||||
|
}
|
||||||
|
// The two seed-INDEPENDENT arms, which is the whole complaint.
|
||||||
|
zeroSimDefault := d.TasteOverlap + d.RandomFill
|
||||||
|
zeroSimSongsLike := s.TasteOverlap + s.RandomFill
|
||||||
|
if zeroSimSongsLike >= zeroSimDefault {
|
||||||
|
t.Errorf("seed-independent arms total %d, not reduced from the default %d; "+
|
||||||
|
"these carry sim_score 0 by construction", zeroSimSongsLike, zeroSimDefault)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// RULE 131: a system playlist degrades, it never vanishes.
|
||||||
|
//
|
||||||
|
// Zeroing the seed-independent arms was the first instinct and is exactly the
|
||||||
|
// vanish-or-nothing shape that rule forbids: a seed whose artist has thin
|
||||||
|
// ListenBrainz coverage would yield a short mix or none at all. They are the
|
||||||
|
// tier-3 FLOOR — reduced hard, never removed — and the songs_like weights are
|
||||||
|
// what keep them at the bottom of the ranking rather than out of the pool.
|
||||||
|
//
|
||||||
|
// "A few tracks further from the seed than we would like" beats "no playlist".
|
||||||
|
func TestSongsLikeLimits_KeepATierThreeFloor(t *testing.T) {
|
||||||
|
s := SongsLikeCandidateSourceLimits()
|
||||||
|
if s.RandomFill <= 0 {
|
||||||
|
t.Error("RandomFill is zero: a seed with thin similarity coverage now produces " +
|
||||||
|
"a short or empty mix instead of degrading (rule 131)")
|
||||||
|
}
|
||||||
|
if s.TasteOverlap <= 0 {
|
||||||
|
t.Error("TasteOverlap is zero: the graded floor is gone, leaving only random " +
|
||||||
|
"fill between a sparse seed and an empty playlist (rule 131)")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// The pool should stay roughly the size it was — this change is about
|
||||||
|
// COMPOSITION, not about starving the surface. A much smaller pool would also
|
||||||
|
// shrink what the per-artist cap has to work with.
|
||||||
|
func TestSongsLikeLimits_KeepThePoolRoughlyTheSameSize(t *testing.T) {
|
||||||
|
total := func(l CandidateSourceLimits) int {
|
||||||
|
return l.LBSimilar + l.SimilarArtist + l.TagOverlap + l.LikesOverlap +
|
||||||
|
l.RandomFill + l.TasteOverlap + l.UserCoplay
|
||||||
|
}
|
||||||
|
d, s := total(DefaultCandidateSourceLimits()), total(SongsLikeCandidateSourceLimits())
|
||||||
|
if s < d/2 {
|
||||||
|
t.Errorf("songs_like pool is %d against the default %d — less than half; "+
|
||||||
|
"this was meant to re-weight the pool, not starve it", s, d)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// The constraint that is invisible in the numbers, and that this file exists
|
||||||
|
// to keep visible.
|
||||||
|
//
|
||||||
|
// `likes_overlap` and `random_fill` end in a bare `ORDER BY random()` with no
|
||||||
|
// daily seed (recommendation.sql:118, :161). Such an arm returns a stable set
|
||||||
|
// only while its LIMIT exceeds the eligible rows; below that it returns a
|
||||||
|
// random SUBSET that differs between two builds on the same day, and the
|
||||||
|
// daily-determinism promise quietly stops holding.
|
||||||
|
//
|
||||||
|
// This is not hypothetical — it is how this change first failed CI. Cutting
|
||||||
|
// RandomFill to 10 broke TestBuildSystemPlaylists_DailyNonceDeterminism,
|
||||||
|
// whose library is smaller than the default limit and whose determinism was
|
||||||
|
// therefore an accident of the limit exceeding the library.
|
||||||
|
//
|
||||||
|
// Growing these arms is always safe. Only shrinking is, and the fix that
|
||||||
|
// would make shrinking safe is a seeded ordering (#3889), not a smaller
|
||||||
|
// number here.
|
||||||
|
func TestSongsLikeLimits_DoNotShrinkTheUnseededRandomArms(t *testing.T) {
|
||||||
|
d := DefaultCandidateSourceLimits()
|
||||||
|
s := SongsLikeCandidateSourceLimits()
|
||||||
|
|
||||||
|
for _, tc := range []struct {
|
||||||
|
arm string
|
||||||
|
songsLike, dflt int
|
||||||
|
}{
|
||||||
|
{"RandomFill", s.RandomFill, d.RandomFill},
|
||||||
|
{"LikesOverlap", s.LikesOverlap, d.LikesOverlap},
|
||||||
|
} {
|
||||||
|
if tc.songsLike < tc.dflt {
|
||||||
|
t.Errorf("%s cut from %d to %d. That arm is ordered by unseeded random(), "+
|
||||||
|
"so a smaller limit makes pool membership vary between same-day "+
|
||||||
|
"rebuilds — it breaks daily determinism rather than merely narrowing "+
|
||||||
|
"the mix. Fix the ordering (#3889) before trimming this.",
|
||||||
|
tc.arm, tc.dflt, tc.songsLike)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -40,6 +40,13 @@ const (
|
|||||||
// never be read as taste signal (#2374) — filing it under taste would put
|
// never be read as taste signal (#2374) — filing it under taste would put
|
||||||
// it one careless join from the leak that design forbids.
|
// it one careless join from the leak that design forbids.
|
||||||
ScopeDiscover = "discover"
|
ScopeDiscover = "discover"
|
||||||
|
// ScopeSongsLike is the "Songs like {X}" surface (#3881). It shared
|
||||||
|
// daily_mix with For-You until 2026-09-10, and that sharing WAS the bug:
|
||||||
|
// the two surfaces want opposite things. For-You answers "what will they
|
||||||
|
// enjoy today" and is supposed to roam; Songs-like answers "what sounds
|
||||||
|
// like THIS" and is the tightest surface in the product. One set of
|
||||||
|
// weights cannot serve both, and the broad answer was winning.
|
||||||
|
ScopeSongsLike = "songs_like"
|
||||||
)
|
)
|
||||||
|
|
||||||
// TasteTuning is the tunable subset of taste.Config: the engagement
|
// TasteTuning is the tunable subset of taste.Config: the engagement
|
||||||
@@ -92,6 +99,76 @@ func ShippedDailyMixWeights() recommendation.ScoringWeights {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ShippedSongsLikeWeights are the shipped songs_like-profile defaults
|
||||||
|
// (#3881). The whole point is that SIMILARITY DOMINATES; every other
|
||||||
|
// profile balances it against taste and engagement, and this one must not.
|
||||||
|
//
|
||||||
|
// The failure being corrected, arithmetic from the daily_mix profile that
|
||||||
|
// this surface used to share:
|
||||||
|
//
|
||||||
|
// unrelated track, liked, not played recently → 1.0 + 2.0 + 1.0 = 4.0
|
||||||
|
// PERFECT similarity match, not liked → 1.0 + 1.5 = 2.5
|
||||||
|
//
|
||||||
|
// Liking something outranked sounding like the seed, because LikeBoost (2.0)
|
||||||
|
// exceeded SimilarityWeight's entire range (1.5) and TasteWeight (1.5, and
|
||||||
|
// seed-independent) matched it outright.
|
||||||
|
//
|
||||||
|
// THE PROPERTY THESE NUMBERS ENCODE, which is what to preserve if they are
|
||||||
|
// retuned: the similarity term's range must exceed the combined range of
|
||||||
|
// every seed-INDEPENDENT differentiator, so that a closer match cannot be
|
||||||
|
// beaten on the strength of likes, freshness and taste alone.
|
||||||
|
//
|
||||||
|
// seed-independent spread = LikeBoost 0.5 + Recency 0.25
|
||||||
|
// + Taste 0.25 + ContextTime 0.5
|
||||||
|
// + jitter 0.05 = 1.55
|
||||||
|
// similarity spread = 0 → 4.0
|
||||||
|
//
|
||||||
|
// So a similarity advantage of ~0.39 (1.55/4.0) wins outright regardless of
|
||||||
|
// everything else, while tracks within that band still get ordered by what
|
||||||
|
// the user likes and has not heard lately. Tight, not deaf.
|
||||||
|
//
|
||||||
|
// BaseWeight stays 1.0: it is identical for every candidate and so
|
||||||
|
// differentiates nothing — it sets the floor, not the shape. SkipPenalty
|
||||||
|
// stays 2.0 because a track the user skips is still unwanted no matter how
|
||||||
|
// similar it is.
|
||||||
|
//
|
||||||
|
// These are DEFAULTS, not settings (rule 25) — the operator turns them in the
|
||||||
|
// admin tuning card and good values get baked back here. They are a
|
||||||
|
// defensible starting point rather than a measured optimum: the per-arm fill
|
||||||
|
// rates and the real sim_score distribution are still unknown (#3879), and
|
||||||
|
// `likes_overlap` contributes a flat 0.6 that a high SimilarityWeight
|
||||||
|
// amplifies. Expect to move these once that lands.
|
||||||
|
func ShippedSongsLikeWeights() recommendation.ScoringWeights {
|
||||||
|
return recommendation.ScoringWeights{
|
||||||
|
BaseWeight: 1.0, // same for all candidates; differentiates nothing
|
||||||
|
LikeBoost: 0.5, // was 2.0 — a tie-break among similar tracks, not an override
|
||||||
|
RecencyWeight: 0.25, // was 1.0 — freshness must not outrank sounding right
|
||||||
|
SkipPenalty: 2.0, // unchanged — a skipped track stays unwanted
|
||||||
|
JitterMagnitude: 0.05, // was 0.1 — less shuffle on a coherence surface
|
||||||
|
ContextWeight: 0.5,
|
||||||
|
SimilarityWeight: 4.0, // was 1.5 — dominant, by design
|
||||||
|
TasteWeight: 0.25, // was 1.5 — seed-INDEPENDENT, so demoted hard
|
||||||
|
ContextTimeWeight: 0.5, // was 1.0
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// shippedWeightsFor returns the shipped defaults for a weight-profile scope,
|
||||||
|
// or ok=false if the scope is not a weight profile. Single source for the
|
||||||
|
// three call sites (seed, update-validation, reset) so adding a fourth
|
||||||
|
// profile cannot be half-wired — which is how a scope ends up seedable but
|
||||||
|
// not resettable.
|
||||||
|
func shippedWeightsFor(scope string) (recommendation.ScoringWeights, bool) {
|
||||||
|
switch scope {
|
||||||
|
case ScopeRadio:
|
||||||
|
return ShippedRadioWeights(), true
|
||||||
|
case ScopeDailyMix:
|
||||||
|
return ShippedDailyMixWeights(), true
|
||||||
|
case ScopeSongsLike:
|
||||||
|
return ShippedSongsLikeWeights(), true
|
||||||
|
}
|
||||||
|
return recommendation.ScoringWeights{}, false
|
||||||
|
}
|
||||||
|
|
||||||
// DiscoverTuning is the tunable set for the Discover request surface (#2377).
|
// DiscoverTuning is the tunable set for the Discover request surface (#2377).
|
||||||
type DiscoverTuning struct {
|
type DiscoverTuning struct {
|
||||||
// TagOverlapWeight scales the taste-tag term: score × (1 + w × overlap).
|
// TagOverlapWeight scales the taste-tag term: score × (1 + w × overlap).
|
||||||
@@ -165,8 +242,9 @@ func New(ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger) (*Service
|
|||||||
func (s *Service) reconcile(ctx context.Context) error {
|
func (s *Service) reconcile(ctx context.Context) error {
|
||||||
q := dbq.New(s.pool)
|
q := dbq.New(s.pool)
|
||||||
for profile, w := range map[string]recommendation.ScoringWeights{
|
for profile, w := range map[string]recommendation.ScoringWeights{
|
||||||
ScopeRadio: ShippedRadioWeights(),
|
ScopeRadio: ShippedRadioWeights(),
|
||||||
ScopeDailyMix: ShippedDailyMixWeights(),
|
ScopeDailyMix: ShippedDailyMixWeights(),
|
||||||
|
ScopeSongsLike: ShippedSongsLikeWeights(),
|
||||||
} {
|
} {
|
||||||
if err := q.UpsertWeightProfileDefaults(ctx, upsertParams(profile, w)); err != nil {
|
if err := q.UpsertWeightProfileDefaults(ctx, upsertParams(profile, w)); err != nil {
|
||||||
return fmt.Errorf("seed profile %q: %w", profile, err)
|
return fmt.Errorf("seed profile %q: %w", profile, err)
|
||||||
@@ -234,6 +312,7 @@ func (s *Service) reconcile(ctx context.Context) error {
|
|||||||
// reads Weights(ScopeRadio) per request.
|
// reads Weights(ScopeRadio) per request.
|
||||||
func (s *Service) push() {
|
func (s *Service) push() {
|
||||||
playlists.SetSystemMixWeights(s.Weights(ScopeDailyMix))
|
playlists.SetSystemMixWeights(s.Weights(ScopeDailyMix))
|
||||||
|
playlists.SetSongsLikeWeights(s.Weights(ScopeSongsLike))
|
||||||
playlists.SetTasteConfig(s.TasteConfig())
|
playlists.SetTasteConfig(s.TasteConfig())
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -297,7 +376,7 @@ type fieldChange struct {
|
|||||||
// Unknown fields and out-of-range values reject the whole patch. A
|
// Unknown fields and out-of-range values reject the whole patch. A
|
||||||
// no-op patch (all values equal to current) writes no audit row.
|
// no-op patch (all values equal to current) writes no audit row.
|
||||||
func (s *Service) UpdateProfile(ctx context.Context, profile string, patch map[string]float64) error {
|
func (s *Service) UpdateProfile(ctx context.Context, profile string, patch map[string]float64) error {
|
||||||
if profile != ScopeRadio && profile != ScopeDailyMix {
|
if _, ok := shippedWeightsFor(profile); !ok {
|
||||||
return fmt.Errorf("%w: %q", ErrUnknownScope, profile)
|
return fmt.Errorf("%w: %q", ErrUnknownScope, profile)
|
||||||
}
|
}
|
||||||
current := s.Weights(profile)
|
current := s.Weights(profile)
|
||||||
@@ -340,17 +419,18 @@ func (s *Service) UpdateDiscover(ctx context.Context, patch map[string]float64)
|
|||||||
// Reset restores a scope to its shipped defaults, with one audit row
|
// Reset restores a scope to its shipped defaults, with one audit row
|
||||||
// carrying the full diff. A scope already at defaults is a no-op.
|
// carrying the full diff. A scope already at defaults is a no-op.
|
||||||
func (s *Service) Reset(ctx context.Context, scope string) error {
|
func (s *Service) Reset(ctx context.Context, scope string) error {
|
||||||
switch scope {
|
if shipped, ok := shippedWeightsFor(scope); ok {
|
||||||
case ScopeRadio, ScopeDailyMix:
|
// Every weight profile resets the same way; the per-scope defaults
|
||||||
shipped := ShippedRadioWeights()
|
// come from one place so a new profile cannot be seedable but not
|
||||||
if scope == ScopeDailyMix {
|
// resettable. This was an if/else over two hard-coded scopes until
|
||||||
shipped = ShippedDailyMixWeights()
|
// songs_like made it three.
|
||||||
}
|
|
||||||
changes := diffWeights(s.Weights(scope), shipped)
|
changes := diffWeights(s.Weights(scope), shipped)
|
||||||
if len(changes) == 0 {
|
if len(changes) == 0 {
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
return s.persistProfile(ctx, scope, shipped, "reset", changes)
|
return s.persistProfile(ctx, scope, shipped, "reset", changes)
|
||||||
|
}
|
||||||
|
switch scope {
|
||||||
case ScopeTaste:
|
case ScopeTaste:
|
||||||
shipped := ShippedTasteTuning()
|
shipped := ShippedTasteTuning()
|
||||||
changes := diffTaste(s.Taste(), shipped)
|
changes := diffTaste(s.Taste(), shipped)
|
||||||
|
|||||||
@@ -0,0 +1,130 @@
|
|||||||
|
package recsettings
|
||||||
|
|
||||||
|
import (
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
|
"git.fabledsword.com/bvandeusen/minstrel/internal/recommendation"
|
||||||
|
)
|
||||||
|
|
||||||
|
// The bug, reproduced as a ranking: a track that sounds nothing like the seed
|
||||||
|
// but which the user liked and has not played lately used to OUTRANK a perfect
|
||||||
|
// similarity match. Operator, 2026-09-10: "I was getting a seeming wide variety
|
||||||
|
// of music from each one when I was hoping to stay in a certain neighborhood."
|
||||||
|
//
|
||||||
|
// This is the whole point of the songs_like profile, so it is asserted as
|
||||||
|
// BEHAVIOUR — two candidates, which one wins — rather than by checking the
|
||||||
|
// weight numbers. Numbers get retuned; this property must survive that.
|
||||||
|
//
|
||||||
|
// It also pins the contrast: daily_mix is EXPECTED to fail this. If both
|
||||||
|
// profiles started ranking the same way, the split would have quietly become
|
||||||
|
// pointless and nothing else would notice.
|
||||||
|
func TestSongsLikeWeights_SimilarityBeatsAnUnrelatedLikedTrack(t *testing.T) {
|
||||||
|
now := time.Now().UTC()
|
||||||
|
stale := now.Add(-365 * 24 * time.Hour)
|
||||||
|
|
||||||
|
// A perfect similarity match the user has never liked and played recently.
|
||||||
|
// Everything except similarity is working against it.
|
||||||
|
perfectMatch := recommendation.ScoringInputs{
|
||||||
|
SimilarityScore: 1.0,
|
||||||
|
IsGeneralLiked: false,
|
||||||
|
LastPlayedAt: &now,
|
||||||
|
}
|
||||||
|
// Nothing to do with the seed, but liked and long unplayed — every
|
||||||
|
// seed-independent term in its favour.
|
||||||
|
unrelatedFavourite := recommendation.ScoringInputs{
|
||||||
|
SimilarityScore: 0.0,
|
||||||
|
IsGeneralLiked: true,
|
||||||
|
LastPlayedAt: &stale,
|
||||||
|
TasteMatchScore: 1.0,
|
||||||
|
}
|
||||||
|
|
||||||
|
// Jitter fixed at its midpoint so the comparison is about the weights.
|
||||||
|
noJitter := func() float64 { return 0.5 }
|
||||||
|
|
||||||
|
songsLike := ShippedSongsLikeWeights()
|
||||||
|
matchScore := recommendation.Score(perfectMatch, songsLike, now, noJitter)
|
||||||
|
favScore := recommendation.Score(unrelatedFavourite, songsLike, now, noJitter)
|
||||||
|
if matchScore <= favScore {
|
||||||
|
t.Errorf("songs_like ranks an unrelated liked track (%.3f) at or above a "+
|
||||||
|
"perfect similarity match (%.3f) — the mix will wander", favScore, matchScore)
|
||||||
|
}
|
||||||
|
|
||||||
|
// The contrast that makes the split worth having. If this ever passes,
|
||||||
|
// daily_mix has been tightened into songs_like and one of them is redundant.
|
||||||
|
daily := ShippedDailyMixWeights()
|
||||||
|
dMatch := recommendation.Score(perfectMatch, daily, now, noJitter)
|
||||||
|
dFav := recommendation.Score(unrelatedFavourite, daily, now, noJitter)
|
||||||
|
if dMatch > dFav {
|
||||||
|
t.Errorf("daily_mix now also puts similarity first (%.3f vs %.3f); the two "+
|
||||||
|
"profiles no longer differ, so songs_like is buying nothing", dMatch, dFav)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// The property the songs_like numbers encode, stated independently of them:
|
||||||
|
// the similarity term's range must exceed the combined range of every
|
||||||
|
// seed-INDEPENDENT differentiator, so a closer match cannot be beaten on
|
||||||
|
// likes, freshness and taste alone.
|
||||||
|
//
|
||||||
|
// BaseWeight is excluded deliberately — it is identical for every candidate
|
||||||
|
// and so differentiates nothing. SkipPenalty is excluded because it only ever
|
||||||
|
// pushes a candidate DOWN, and a skipped track should lose however similar.
|
||||||
|
func TestSongsLikeWeights_SimilarityOutrangesEverySeedIndependentTerm(t *testing.T) {
|
||||||
|
w := ShippedSongsLikeWeights()
|
||||||
|
|
||||||
|
// TasteMatchScore and ContextAffinityScore are in [-1,+1]; recencyDecay is
|
||||||
|
// in [0,1]; LikeBoost is all-or-nothing.
|
||||||
|
seedIndependent := w.LikeBoost + w.RecencyWeight + w.TasteWeight +
|
||||||
|
w.ContextTimeWeight + w.JitterMagnitude
|
||||||
|
|
||||||
|
if w.SimilarityWeight <= seedIndependent {
|
||||||
|
t.Errorf("SimilarityWeight %.2f does not outrange the seed-independent "+
|
||||||
|
"terms (%.2f) — likes/recency/taste can outvote sounding like the seed",
|
||||||
|
w.SimilarityWeight, seedIndependent)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// A scope that is seedable but not resettable is the half-wired shape this
|
||||||
|
// guards: the profile appears in the admin card, the operator turns a knob,
|
||||||
|
// and Reset then 404s on a scope the rest of the service knows about.
|
||||||
|
func TestShippedWeightsFor_CoversEveryWeightProfile(t *testing.T) {
|
||||||
|
for _, scope := range []string{ScopeRadio, ScopeDailyMix, ScopeSongsLike} {
|
||||||
|
if _, ok := shippedWeightsFor(scope); !ok {
|
||||||
|
t.Errorf("scope %q has no shipped defaults; it cannot be seeded or reset", scope)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// Non-weight scopes must NOT resolve here, or Reset would treat the taste
|
||||||
|
// singleton as a weight profile and write nonsense.
|
||||||
|
for _, scope := range []string{ScopeTaste, ScopeDiscover, "nonsense"} {
|
||||||
|
if _, ok := shippedWeightsFor(scope); ok {
|
||||||
|
t.Errorf("scope %q resolved as a weight profile and is not one", scope)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Guards the sync the comment in playlists/system.go asks for: the pre-push
|
||||||
|
// literal there must match the shipped defaults here, or a build that has not
|
||||||
|
// yet been reconciled ranks differently from one that has.
|
||||||
|
func TestShippedSongsLikeWeights_AreDominatedBySimilarity(t *testing.T) {
|
||||||
|
w := ShippedSongsLikeWeights()
|
||||||
|
daily := ShippedDailyMixWeights()
|
||||||
|
|
||||||
|
if w.SimilarityWeight <= daily.SimilarityWeight {
|
||||||
|
t.Errorf("songs_like SimilarityWeight %.2f is not above daily_mix's %.2f",
|
||||||
|
w.SimilarityWeight, daily.SimilarityWeight)
|
||||||
|
}
|
||||||
|
for _, tc := range []struct {
|
||||||
|
name string
|
||||||
|
songs, day float64
|
||||||
|
}{
|
||||||
|
{"LikeBoost", w.LikeBoost, daily.LikeBoost},
|
||||||
|
{"TasteWeight", w.TasteWeight, daily.TasteWeight},
|
||||||
|
{"RecencyWeight", w.RecencyWeight, daily.RecencyWeight},
|
||||||
|
} {
|
||||||
|
if tc.songs >= tc.day {
|
||||||
|
t.Errorf("songs_like %s (%.2f) is not demoted below daily_mix (%.2f); "+
|
||||||
|
"these are the seed-independent terms that made the mix wander",
|
||||||
|
tc.name, tc.songs, tc.day)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1,718 @@
|
|||||||
|
package server
|
||||||
|
|
||||||
|
import (
|
||||||
|
"os"
|
||||||
|
"os/exec"
|
||||||
|
"path/filepath"
|
||||||
|
"regexp"
|
||||||
|
"strconv"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
|
// Guards the version derivation that stamps every build.
|
||||||
|
//
|
||||||
|
// These assertions used to be impossible to run. The derivation lived inline
|
||||||
|
// in release.yml, which triggers only on `main` and on tags — so a mistake in
|
||||||
|
// it could not surface until a release was already under way, and its failure
|
||||||
|
// mode is silence: a version nobody can compare looks exactly like being up to
|
||||||
|
// date, and nobody reports an update they were never offered.
|
||||||
|
//
|
||||||
|
// Moving it to ci/version.sh made it executable, so this runs on every push
|
||||||
|
// that touches the release machinery. That is the whole point of the file; the
|
||||||
|
// specific assertions below matter less than the fact that they run at all.
|
||||||
|
//
|
||||||
|
// The tests EXECUTE the script rather than asserting on its text, so they
|
||||||
|
// break when the behaviour changes rather than when the wording does.
|
||||||
|
|
||||||
|
// highestOldSchemeCode is the largest versionCode ever shipped under the
|
||||||
|
// retired commit-count scheme (v2026.09.09 shipped 1895). Every code the new
|
||||||
|
// scheme emits must clear it, or Android would refuse the upgrade as a
|
||||||
|
// downgrade and the update channel would be a one-way door.
|
||||||
|
const highestOldSchemeCode = 1895
|
||||||
|
|
||||||
|
func repoRoot(t *testing.T) string {
|
||||||
|
t.Helper()
|
||||||
|
dir, err := os.Getwd()
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
for {
|
||||||
|
if _, err := os.Stat(filepath.Join(dir, "go.mod")); err == nil {
|
||||||
|
return dir
|
||||||
|
}
|
||||||
|
parent := filepath.Dir(dir)
|
||||||
|
if parent == dir {
|
||||||
|
t.Fatalf("no go.mod above %s", dir)
|
||||||
|
}
|
||||||
|
dir = parent
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// runVersion executes ci/version.sh with both clocks pinned, so the result is
|
||||||
|
// deterministic. Returns the parsed KEY=VALUE output.
|
||||||
|
func runVersion(t *testing.T, commitEpoch, nowEpoch string) map[string]string {
|
||||||
|
t.Helper()
|
||||||
|
out, err := versionScript(t, commitEpoch, nowEpoch)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("ci/version.sh failed: %v\n%s", err, out)
|
||||||
|
}
|
||||||
|
parsed := map[string]string{}
|
||||||
|
for _, line := range strings.Split(strings.TrimSpace(out), "\n") {
|
||||||
|
if k, v, ok := strings.Cut(line, "="); ok {
|
||||||
|
parsed[k] = v
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return parsed
|
||||||
|
}
|
||||||
|
|
||||||
|
func versionScript(t *testing.T, commitEpoch, nowEpoch string) (string, error) {
|
||||||
|
t.Helper()
|
||||||
|
root := repoRoot(t)
|
||||||
|
cmd := exec.Command(filepath.Join(root, "ci", "version.sh"))
|
||||||
|
cmd.Dir = root
|
||||||
|
cmd.Env = append(os.Environ(),
|
||||||
|
"MINSTREL_COMMIT_EPOCH="+commitEpoch,
|
||||||
|
"MINSTREL_NOW_EPOCH="+nowEpoch,
|
||||||
|
)
|
||||||
|
out, err := cmd.CombinedOutput()
|
||||||
|
return string(out), err
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestVersionName_IsCommitTimeToTheMinute(t *testing.T) {
|
||||||
|
// 2025-09-09T18:48:56Z
|
||||||
|
got := runVersion(t, "1757443736", "1789000920")
|
||||||
|
if want := "2025.09.09.1848"; got["name"] != want {
|
||||||
|
t.Errorf("name = %q, want %q", got["name"], want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// HHMM is the segment most likely to be silently mangled, and it only bites
|
||||||
|
// for about a tenth of the day — a build just after midnight must emit "0042",
|
||||||
|
// never "42". A stripped leading zero shifts the segment by two orders of
|
||||||
|
// magnitude and reverses comparisons against every other build that day.
|
||||||
|
func TestVersionName_PadsTheMinuteSegment(t *testing.T) {
|
||||||
|
// 2026-09-10T00:42:00Z
|
||||||
|
got := runVersion(t, "1789000920", "1789000920")
|
||||||
|
if want := "2026.09.10.0042"; got["name"] != want {
|
||||||
|
t.Errorf("name = %q, want %q — leading zero lost?", got["name"], want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestVersionName_DerivesFromCommitNotBuildClock(t *testing.T) {
|
||||||
|
// Same commit, two different build clocks: the NAME must not move, or a
|
||||||
|
// dev build and a main build of one commit would report different strings
|
||||||
|
// and the channel field would stop being the only thing separating them.
|
||||||
|
a := runVersion(t, "1757443736", "1789000920")
|
||||||
|
b := runVersion(t, "1757443736", "1789500000")
|
||||||
|
if a["name"] != b["name"] {
|
||||||
|
t.Errorf("name moved with the build clock: %q vs %q", a["name"], b["name"])
|
||||||
|
}
|
||||||
|
if a["code"] == b["code"] {
|
||||||
|
t.Errorf("code did NOT move with the build clock (%q) — it is not build-derived", a["code"])
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestVersionCode_IsMinutesSince2020AndClearsTheOldScheme(t *testing.T) {
|
||||||
|
got := runVersion(t, "1789000920", "1789000920")
|
||||||
|
code, err := strconv.Atoi(got["code"])
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("code %q is not an integer: %v", got["code"], err)
|
||||||
|
}
|
||||||
|
if want := (1789000920 - 1577836800) / 60; code != want {
|
||||||
|
t.Errorf("code = %d, want %d", code, want)
|
||||||
|
}
|
||||||
|
if code <= highestOldSchemeCode {
|
||||||
|
t.Errorf("code %d does not clear the retired commit-count scheme (%d) — "+
|
||||||
|
"Android would refuse the upgrade as a downgrade", code, highestOldSchemeCode)
|
||||||
|
}
|
||||||
|
if int64(code) > 2147483647 {
|
||||||
|
t.Errorf("code %d overflows versionCode's int32 ceiling", code)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// The tag is not chosen, it is the name with a `v`. Anything else reintroduces
|
||||||
|
// the mismatch between what a tag claims and what the artifact reports.
|
||||||
|
func TestTag_IsTheNameWithAPrefix(t *testing.T) {
|
||||||
|
got := runVersion(t, "1757443736", "1789000920")
|
||||||
|
if want := "v" + got["name"]; got["tag"] != want {
|
||||||
|
t.Errorf("tag = %q, want %q", got["tag"], want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// A guard that cannot fail is worse than no guard. These prove the script
|
||||||
|
// rejects the shapes it claims to reject, rather than emitting something
|
||||||
|
// plausible and letting it ship.
|
||||||
|
func TestVersionScript_RejectsUnusableClocks(t *testing.T) {
|
||||||
|
for _, tc := range []struct{ name, commit, now string }{
|
||||||
|
{"unreadable commit timestamp", "notanumber", "1789000920"},
|
||||||
|
{"non-numeric build clock", "1789000920", "abc"},
|
||||||
|
{"build clock before 2020", "1789000920", "1000000000"},
|
||||||
|
} {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
out, err := versionScript(t, tc.commit, tc.now)
|
||||||
|
if err == nil {
|
||||||
|
t.Errorf("script succeeded on %s, output: %s", tc.name, out)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Pins the wiring, not the formula: if release.yml stops calling the script,
|
||||||
|
// every assertion above keeps passing while the thing that actually ships goes
|
||||||
|
// unguarded again. That silent decoupling is the specific regression here.
|
||||||
|
func TestReleaseWorkflow_UsesTheSharedDerivation(t *testing.T) {
|
||||||
|
body, err := os.ReadFile(filepath.Join(repoRoot(t), ".gitea", "workflows", "release.yml"))
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
yaml := string(body)
|
||||||
|
|
||||||
|
if !strings.Contains(yaml, "ci/version.sh") {
|
||||||
|
t.Error("release.yml no longer calls ci/version.sh — the derivation has drifted out of test coverage")
|
||||||
|
}
|
||||||
|
// The retired scheme, which must not come back. A presence check is safe
|
||||||
|
// against prose; the historical note in that file names the old formula
|
||||||
|
// only in comments, so match the executable form.
|
||||||
|
if strings.Contains(yaml, "$(git rev-list --count") {
|
||||||
|
t.Error("release.yml derives a commit count again — that is not monotonic across branches")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// jobBoundary matches a blank line followed by job-level (two-space)
|
||||||
|
// indentation — the end of the last step in a job.
|
||||||
|
var jobBoundary = regexp.MustCompile(`\n\n [^ \n]`)
|
||||||
|
|
||||||
|
// stepBody returns one workflow step's text, from its `- name:` line to the
|
||||||
|
// start of the next step or the next job.
|
||||||
|
//
|
||||||
|
// The naive cut — "up to the next `- name:`" — silently returns an EMPTY body
|
||||||
|
// for the last step in a job, and every assertion over it then passes
|
||||||
|
// vacuously. That is the failure rule 167 names: a check that reads as
|
||||||
|
// coverage while asserting on nothing. Cutting at a blank line followed by
|
||||||
|
// job-level indentation handles the last-step case, which is exactly where
|
||||||
|
// `Build and push` sits.
|
||||||
|
func stepBody(t *testing.T, yaml, name string) string {
|
||||||
|
t.Helper()
|
||||||
|
i := strings.Index(yaml, "- name: "+name)
|
||||||
|
if i < 0 {
|
||||||
|
t.Fatalf("no %q step in release.yml", name)
|
||||||
|
}
|
||||||
|
body := yaml[i:]
|
||||||
|
end := len(body)
|
||||||
|
if j := strings.Index(body, "\n - name:"); j >= 0 {
|
||||||
|
end = j
|
||||||
|
}
|
||||||
|
// A blank line followed by EXACTLY two spaces and then content starts a
|
||||||
|
// new job or job-level comment. The "exactly" matters: a blank line inside
|
||||||
|
// a `run:` block is followed by ten-space indentation and would otherwise
|
||||||
|
// match, truncating the step mid-body — which is how this helper first
|
||||||
|
// sliced the verify step down to its first two lines.
|
||||||
|
if loc := jobBoundary.FindStringIndex(body); loc != nil && loc[0] < end {
|
||||||
|
end = loc[0]
|
||||||
|
}
|
||||||
|
body = body[:end]
|
||||||
|
if strings.TrimSpace(strings.TrimPrefix(body, "- name: "+name)) == "" {
|
||||||
|
t.Fatalf("step %q sliced to an empty body — the assertions below would pass vacuously", name)
|
||||||
|
}
|
||||||
|
return body
|
||||||
|
}
|
||||||
|
|
||||||
|
// releaseYAML reads the workflow once per test.
|
||||||
|
func releaseYAML(t *testing.T) string {
|
||||||
|
t.Helper()
|
||||||
|
body, err := os.ReadFile(filepath.Join(repoRoot(t), ".gitea", "workflows", "release.yml"))
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
return string(body)
|
||||||
|
}
|
||||||
|
|
||||||
|
// imageTagArms splits "Compute image tags" into its three ref branches. The
|
||||||
|
// whole tag policy lives in that if/elif/else, so the arms are the unit worth
|
||||||
|
// asserting on — a tag published from the wrong arm reaches the wrong
|
||||||
|
// audience, and every such mistake still builds and still pushes a valid
|
||||||
|
// image.
|
||||||
|
func imageTagArms(t *testing.T, yaml string) (tagArm, devArm, mainArm string) {
|
||||||
|
t.Helper()
|
||||||
|
const (
|
||||||
|
tagMarker = `if [[ "${GITHUB_REF}" == refs/tags/v* ]]; then`
|
||||||
|
devMarker = `elif [[ "${GITHUB_REF}" == "refs/heads/dev" ]]; then`
|
||||||
|
mainMarker = "\n else"
|
||||||
|
endMarker = "\n fi"
|
||||||
|
)
|
||||||
|
iTag := strings.Index(yaml, tagMarker)
|
||||||
|
iDev := strings.Index(yaml, devMarker)
|
||||||
|
iMain := -1
|
||||||
|
if iDev >= 0 {
|
||||||
|
if k := strings.Index(yaml[iDev:], mainMarker); k >= 0 {
|
||||||
|
iMain = iDev + k
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if iTag < 0 || iDev < 0 || iMain < 0 {
|
||||||
|
t.Fatal("Compute image tags no longer has a tag/dev/main arm — the tag policy has been restructured, so these guards are pinning nothing")
|
||||||
|
}
|
||||||
|
iEnd := iMain
|
||||||
|
if k := strings.Index(yaml[iMain:], endMarker); k >= 0 {
|
||||||
|
iEnd = iMain + k
|
||||||
|
} else {
|
||||||
|
t.Fatal("no closing fi after the main arm")
|
||||||
|
}
|
||||||
|
return yaml[iTag:iDev], yaml[iDev:iMain], yaml[iMain:iEnd]
|
||||||
|
}
|
||||||
|
|
||||||
|
// The worst regression this wiring can produce: a dev push that also moves
|
||||||
|
// :latest would ship untested code to every stable operator, silently, on the
|
||||||
|
// next pull. Nothing else in the suite would notice — the build stays green
|
||||||
|
// and the image is valid, it is simply the wrong audience.
|
||||||
|
func TestDevChannel_PublishesDevAloneAndNeverLatest(t *testing.T) {
|
||||||
|
_, arm, _ := imageTagArms(t, releaseYAML(t))
|
||||||
|
|
||||||
|
if !strings.Contains(arm, "${IMAGE}:dev") {
|
||||||
|
t.Errorf("dev arm does not publish :dev\n%s", arm)
|
||||||
|
}
|
||||||
|
if strings.Contains(arm, ":latest") {
|
||||||
|
t.Errorf("dev arm moves :latest — that ships dev code to every stable operator\n%s", arm)
|
||||||
|
}
|
||||||
|
// Rule 145: a rolling channel gets no commit-addressable tag.
|
||||||
|
if strings.Contains(arm, "GITHUB_SHA") {
|
||||||
|
t.Errorf("dev arm publishes a per-commit tag; a rolling channel should not\n%s", arm)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// The two bundling paths must stay mutually exclusive. If the rebundle step's
|
||||||
|
// condition were relaxed back to "not a tag", a dev push would run BOTH: stage
|
||||||
|
// its freshly-built APK, then overwrite it with the previous release's. The
|
||||||
|
// image would still build and the sidecar would still parse — it would just
|
||||||
|
// quietly serve stale art to the channel whose whole job is being current.
|
||||||
|
func TestDevChannel_RebundlePathIsMainOnly(t *testing.T) {
|
||||||
|
body, err := os.ReadFile(filepath.Join(repoRoot(t), ".gitea", "workflows", "release.yml"))
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
yaml := string(body)
|
||||||
|
|
||||||
|
i := strings.Index(yaml, "- name: Bundle latest release APK")
|
||||||
|
if i < 0 {
|
||||||
|
t.Fatal("no 'Bundle latest release APK' step")
|
||||||
|
}
|
||||||
|
step := yaml[i:]
|
||||||
|
if j := strings.Index(step, "\n - name:"); j >= 0 {
|
||||||
|
step = step[:j]
|
||||||
|
}
|
||||||
|
if !strings.Contains(step, "github.ref == 'refs/heads/main'") {
|
||||||
|
t.Errorf("the rebundle step is not gated to main; a dev push would overwrite its own APK\n%s", step)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// The runner invokes `shell: bash` as `bash -e -o pipefail`. That makes any
|
||||||
|
// command substitution whose grep matches NOTHING fatal at the assignment —
|
||||||
|
// pipefail reports the grep's non-zero status even though `head` succeeded —
|
||||||
|
// so the step dies before reaching the `if` written to handle the empty case.
|
||||||
|
//
|
||||||
|
// This is not hypothetical. It took down the first `main` build after the
|
||||||
|
// version rework: the newest release predated sidecar assets, its
|
||||||
|
// `.apk.version` grep matched nothing, and the step aborted instead of
|
||||||
|
// falling through to the name-only branch that exists precisely for it. The
|
||||||
|
// same latent hazard sat on the other two assignments and had simply never
|
||||||
|
// fired, because their greps always matched.
|
||||||
|
//
|
||||||
|
// Pins the property that makes every "degrades gracefully" branch in that
|
||||||
|
// step reachable at all.
|
||||||
|
func TestBundleStep_GrepsCannotKillTheStep(t *testing.T) {
|
||||||
|
body, err := os.ReadFile(filepath.Join(repoRoot(t), ".gitea", "workflows", "release.yml"))
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
yaml := string(body)
|
||||||
|
|
||||||
|
i := strings.Index(yaml, "- name: Bundle latest release APK")
|
||||||
|
if i < 0 {
|
||||||
|
t.Fatal("no 'Bundle latest release APK' step")
|
||||||
|
}
|
||||||
|
step := yaml[i:]
|
||||||
|
if j := strings.Index(step, "\n - name:"); j >= 0 {
|
||||||
|
step = step[:j]
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, line := range strings.Split(step, "\n") {
|
||||||
|
trimmed := strings.TrimSpace(line)
|
||||||
|
if strings.HasPrefix(trimmed, "#") || !strings.Contains(trimmed, "grep") {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
// Only assignments from a command substitution can abort the step.
|
||||||
|
if !strings.Contains(trimmed, `="$(`) {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
if !strings.HasSuffix(trimmed, "|| true") {
|
||||||
|
t.Errorf("this assignment dies under pipefail when its grep matches nothing, "+
|
||||||
|
"skipping the empty-case branch below it:\n %s", trimmed)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// The rollback unit, and the reason it is worth a guard: it is invisible until
|
||||||
|
// the moment it is needed. Nothing pulls :<sha> during normal operation, so if
|
||||||
|
// this arm stopped minting one, every build would stay green and every image
|
||||||
|
// would be valid — and the absence would surface only during an incident, as
|
||||||
|
// "there is nothing to roll back to."
|
||||||
|
//
|
||||||
|
// Rules 145 and 147: main push → :latest + :<sha>.
|
||||||
|
func TestMainChannel_PublishesLatestAndTheRollbackUnit(t *testing.T) {
|
||||||
|
_, _, arm := imageTagArms(t, releaseYAML(t))
|
||||||
|
|
||||||
|
if !strings.Contains(arm, "${IMAGE}:latest") {
|
||||||
|
t.Errorf("main arm does not move :latest — production would stop tracking main's tip\n%s", arm)
|
||||||
|
}
|
||||||
|
if !strings.Contains(arm, "${IMAGE}:${GITHUB_SHA}") {
|
||||||
|
t.Errorf("main arm publishes no commit-addressable image; there is no rollback target for production commits\n%s", arm)
|
||||||
|
}
|
||||||
|
// Rule 147: :latest tracks main's tip, and a second name for the same
|
||||||
|
// image sends readers looking for a distinction that does not exist.
|
||||||
|
if strings.Contains(arm, "${IMAGE}:main") {
|
||||||
|
t.Errorf("main arm publishes :main — rule 147 says that tag should not exist\n%s", arm)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// A release refreshes the CHANNEL and mints nothing else (rules 145 + 146).
|
||||||
|
//
|
||||||
|
// The specific regression: re-adding :<sha> here. The tag build rebuilds the
|
||||||
|
// SAME SOURCE as main's build minutes earlier, differing only in which APK is
|
||||||
|
// baked in — so a :<sha> minted here would overwrite main's immutable rollback
|
||||||
|
// target with different contents, under the same name. That is the exact thing
|
||||||
|
// rule 145's immutability clause exists to prevent, and it is the half with
|
||||||
|
// the incidents behind it.
|
||||||
|
func TestReleaseBuild_RefreshesTheChannelAndMintsNothingElse(t *testing.T) {
|
||||||
|
arm, _, _ := imageTagArms(t, releaseYAML(t))
|
||||||
|
|
||||||
|
if !strings.Contains(arm, "${IMAGE}:latest") {
|
||||||
|
t.Errorf("tag arm does not refresh :latest — the channel would keep serving the PREVIOUS release's APK until someone pushed to main\n%s", arm)
|
||||||
|
}
|
||||||
|
if strings.Contains(arm, "GITHUB_SHA") {
|
||||||
|
t.Errorf("tag arm mints a :<sha> image; that would re-push main's immutable rollback target with different bundled contents\n%s", arm)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// No version-numbered image tags anywhere, on any arm (rule 145, and the
|
||||||
|
// operator's 2026-09-10 decision to drop them across every project).
|
||||||
|
//
|
||||||
|
// Asserted across the whole step rather than per-arm because the mistake this
|
||||||
|
// catches is re-adding one ANYWHERE, and the tag arm is only the likeliest
|
||||||
|
// spot. `${VERSION}` still legitimately appears in the step as the build's
|
||||||
|
// self-reported version, so the assertion has to name the image-tag form
|
||||||
|
// specifically rather than the variable — otherwise it would fire on correct
|
||||||
|
// code and get "fixed" by deleting the guard.
|
||||||
|
func TestNoVersionNumberedImageTags(t *testing.T) {
|
||||||
|
yaml := releaseYAML(t)
|
||||||
|
tagArm, devArm, mainArm := imageTagArms(t, yaml)
|
||||||
|
|
||||||
|
for _, tc := range []struct{ name, arm string }{
|
||||||
|
{"tag", tagArm}, {"dev", devArm}, {"main", mainArm},
|
||||||
|
} {
|
||||||
|
for _, forbidden := range []string{
|
||||||
|
"${IMAGE}:${VERSION}",
|
||||||
|
"${IMAGE}:v",
|
||||||
|
"${IMAGE}:${GITHUB_REF#refs/tags/}",
|
||||||
|
} {
|
||||||
|
if strings.Contains(tc.arm, forbidden) {
|
||||||
|
t.Errorf("%s arm publishes a version-numbered image tag (%q); git and the build's self-reported version answer \"which build is this\"\n%s",
|
||||||
|
tc.name, forbidden, tc.arm)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// The verify job asserted the :<version> image existed. With version tags
|
||||||
|
// gone that assertion would fail every release for a tag nothing mints — so
|
||||||
|
// this pins that it was re-pointed rather than deleted, since deleting it is
|
||||||
|
// the tempting way to make a failing guard go green.
|
||||||
|
func TestVerifyJob_ChecksTheRollbackImageNotAVersionTag(t *testing.T) {
|
||||||
|
yaml := releaseYAML(t)
|
||||||
|
|
||||||
|
if !strings.Contains(yaml, "- name: Rollback image must exist for the tagged commit") {
|
||||||
|
t.Fatal("the release-verification step that checks an image exists is gone; an image push that silently did not happen would now pass verification")
|
||||||
|
}
|
||||||
|
step := stepBody(t, yaml, "Rollback image must exist for the tagged commit")
|
||||||
|
if !strings.Contains(step, "${IMAGE}:${GITHUB_SHA}") {
|
||||||
|
t.Errorf("the verify step does not inspect the commit's rollback image\n%s", step)
|
||||||
|
}
|
||||||
|
if strings.Contains(step, "${IMAGE}:${TAG}") {
|
||||||
|
t.Errorf("the verify step still inspects a version-numbered image, which is no longer published — this would fail every release\n%s", step)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// The server's self-reported version is now the ONLY thing that identifies a
|
||||||
|
// build, so a lane that stamps a channel word instead of a version silently
|
||||||
|
// removes that ability. It used to stamp the literal "main"/"dev".
|
||||||
|
func TestImageBuild_StampsADerivedVersionAndAChannel(t *testing.T) {
|
||||||
|
yaml := releaseYAML(t)
|
||||||
|
|
||||||
|
step := stepBody(t, yaml, "Build and push")
|
||||||
|
for _, want := range []string{
|
||||||
|
"MINSTREL_VERSION=",
|
||||||
|
"MINSTREL_CHANNEL=",
|
||||||
|
} {
|
||||||
|
if !strings.Contains(step, want) {
|
||||||
|
t.Errorf("Build and push does not pass %s — the image cannot report which build it is\n%s", want, step)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// The version must come from the shared derivation, not from the ref.
|
||||||
|
// Reading it off GITHUB_REF is what produced "main" and "dev" as version
|
||||||
|
// strings, which is the regression this pins.
|
||||||
|
_, _, mainArm := imageTagArms(t, yaml)
|
||||||
|
if strings.Contains(mainArm, `version=main`) {
|
||||||
|
t.Errorf("the main arm stamps the literal string \"main\" as a version; two images months apart would be indistinguishable\n%s", mainArm)
|
||||||
|
}
|
||||||
|
if !strings.Contains(yaml, "ci/version.sh HEAD | sed") {
|
||||||
|
t.Error("the image job no longer derives its version from ci/version.sh — the version and the APK's version can now drift apart")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// gitRepo builds a throwaway repo and returns its path. Commit timestamps are
|
||||||
|
// pinned so the derivation is deterministic.
|
||||||
|
func gitRepo(t *testing.T) string {
|
||||||
|
t.Helper()
|
||||||
|
dir := t.TempDir()
|
||||||
|
run := func(args ...string) {
|
||||||
|
t.Helper()
|
||||||
|
cmd := exec.Command("git", args...)
|
||||||
|
cmd.Dir = dir
|
||||||
|
cmd.Env = append(os.Environ(), "GIT_CONFIG_GLOBAL=/dev/null", "GIT_CONFIG_SYSTEM=/dev/null")
|
||||||
|
if out, err := cmd.CombinedOutput(); err != nil {
|
||||||
|
t.Fatalf("git %v: %v\n%s", args, err, out)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
run("-c", "init.defaultBranch=main", "init", "-q")
|
||||||
|
run("config", "user.email", "t@example.invalid")
|
||||||
|
run("config", "user.name", "t")
|
||||||
|
return dir
|
||||||
|
}
|
||||||
|
|
||||||
|
// commitFile writes path and commits it with a pinned committer timestamp.
|
||||||
|
func commitFile(t *testing.T, dir, path, epoch string) {
|
||||||
|
t.Helper()
|
||||||
|
full := filepath.Join(dir, path)
|
||||||
|
if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
// Content must DIFFER from any earlier write to the same path, or git
|
||||||
|
// records nothing and the commit silently covers fewer files than the
|
||||||
|
// test believes. That is not hypothetical: it made the
|
||||||
|
// source-alongside-test case pass vacuously.
|
||||||
|
if err := os.WriteFile(full, []byte("content @"+epoch+"\n"), 0o644); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
for _, args := range [][]string{{"add", "-A"}, {"commit", "-q", "-m", path}} {
|
||||||
|
cmd := exec.Command("git", args...)
|
||||||
|
cmd.Dir = dir
|
||||||
|
cmd.Env = append(os.Environ(),
|
||||||
|
"GIT_CONFIG_GLOBAL=/dev/null", "GIT_CONFIG_SYSTEM=/dev/null",
|
||||||
|
"GIT_AUTHOR_DATE=@"+epoch+" +0000",
|
||||||
|
"GIT_COMMITTER_DATE=@"+epoch+" +0000",
|
||||||
|
)
|
||||||
|
if out, err := cmd.CombinedOutput(); err != nil {
|
||||||
|
t.Fatalf("git %v: %v\n%s", args, err, out)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// versionIn runs ci/version.sh inside dir, reading real git rather than the
|
||||||
|
// pinned-clock override, so the PATHSPEC is what is under test.
|
||||||
|
func versionIn(t *testing.T, dir string) (string, error) {
|
||||||
|
t.Helper()
|
||||||
|
cmd := exec.Command(filepath.Join(repoRoot(t), "ci", "version.sh"), "HEAD")
|
||||||
|
cmd.Dir = dir
|
||||||
|
cmd.Env = append(os.Environ(),
|
||||||
|
"GIT_CONFIG_GLOBAL=/dev/null", "GIT_CONFIG_SYSTEM=/dev/null",
|
||||||
|
"MINSTREL_NOW_EPOCH=1789000920",
|
||||||
|
)
|
||||||
|
out, err := cmd.CombinedOutput()
|
||||||
|
return string(out), err
|
||||||
|
}
|
||||||
|
|
||||||
|
// The version names what SHIPPED, so a commit that changes nothing shippable
|
||||||
|
// must not move it.
|
||||||
|
//
|
||||||
|
// The failure this prevents is not the cosmetic one. The derivation is a
|
||||||
|
// denylist precisely so that new content counts by default: the direction that
|
||||||
|
// matters is a changed artifact keeping its OLD version, silently, on a green
|
||||||
|
// run. This test pins the cheap half of that (CI-only commits are inert) and,
|
||||||
|
// in the same breath, that a source commit still moves it — because a pathspec
|
||||||
|
// typo that excluded everything would satisfy the first assertion alone.
|
||||||
|
func TestVersionName_IgnoresCommitsThatShipNothing(t *testing.T) {
|
||||||
|
const (
|
||||||
|
shipped = "1757443736" // 2025-09-09T18:48:56Z
|
||||||
|
ciOnly = "1789000920" // 2026-09-10T00:42:00Z, later
|
||||||
|
)
|
||||||
|
dir := gitRepo(t)
|
||||||
|
commitFile(t, dir, "internal/server/thing.go", shipped)
|
||||||
|
commitFile(t, dir, ".gitea/workflows/release.yml", ciOnly)
|
||||||
|
|
||||||
|
out, err := versionIn(t, dir)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("version.sh failed: %v\n%s", err, out)
|
||||||
|
}
|
||||||
|
if !strings.Contains(out, "name=2025.09.09.1848") {
|
||||||
|
t.Errorf("a CI-only commit moved the version — the pathspec is not excluding it\n%s", out)
|
||||||
|
}
|
||||||
|
|
||||||
|
// ...and the pathspec must not be so broad it excludes everything.
|
||||||
|
commitFile(t, dir, "internal/server/other.go", ciOnly)
|
||||||
|
out, err = versionIn(t, dir)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("version.sh failed: %v\n%s", err, out)
|
||||||
|
}
|
||||||
|
if !strings.Contains(out, "name=2026.09.10.0042") {
|
||||||
|
t.Errorf("a source commit did NOT move the version — the pathspec excludes too much, which is the silent-lie direction\n%s", out)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// android/ is deliberately NOT excluded, and that is the subtle half of the
|
||||||
|
// list. It ships in no server image — but it is the APK's entire source, and
|
||||||
|
// ONE script derives the version for both artifacts. Excluding it would stop
|
||||||
|
// an Android-only commit from moving the APK's own version, which is exactly
|
||||||
|
// the silent downgrade the versioning rework exists to prevent.
|
||||||
|
func TestVersionName_AndroidSourcesCount(t *testing.T) {
|
||||||
|
dir := gitRepo(t)
|
||||||
|
commitFile(t, dir, "internal/server/thing.go", "1757443736")
|
||||||
|
commitFile(t, dir, "android/app/src/main/Thing.kt", "1789000920")
|
||||||
|
|
||||||
|
out, err := versionIn(t, dir)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("version.sh failed: %v\n%s", err, out)
|
||||||
|
}
|
||||||
|
if !strings.Contains(out, "name=2026.09.10.0042") {
|
||||||
|
t.Errorf("an Android commit did not move the version; the APK would ship new code under its old version name\n%s", out)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// No shipped commit in range means a shallow clone, and the script must refuse
|
||||||
|
// rather than emit something plausible. A wrong version builds, signs and
|
||||||
|
// publishes perfectly happily; it surfaces later as an update channel that has
|
||||||
|
// quietly stopped offering anything.
|
||||||
|
func TestVersionScript_RefusesWhenNothingShippedIsInRange(t *testing.T) {
|
||||||
|
dir := gitRepo(t)
|
||||||
|
commitFile(t, dir, "ci/version.sh", "1789000920")
|
||||||
|
|
||||||
|
out, err := versionIn(t, dir)
|
||||||
|
if err == nil {
|
||||||
|
t.Fatalf("script succeeded with no shipped commit in range; it should refuse\n%s", out)
|
||||||
|
}
|
||||||
|
if strings.Contains(out, "name=") {
|
||||||
|
t.Errorf("script emitted a version name while refusing — that value could still be consumed\n%s", out)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// commitFiles is commitFile for more than one path in a single commit.
|
||||||
|
func commitFiles(t *testing.T, dir, epoch string, paths ...string) {
|
||||||
|
t.Helper()
|
||||||
|
for _, rel := range paths {
|
||||||
|
full := filepath.Join(dir, rel)
|
||||||
|
if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if err := os.WriteFile(full, []byte("content @"+epoch+"\n"), 0o644); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
for _, args := range [][]string{{"add", "-A"}, {"commit", "-q", "-m", strings.Join(paths, " ")}} {
|
||||||
|
cmd := exec.Command("git", args...)
|
||||||
|
cmd.Dir = dir
|
||||||
|
cmd.Env = append(os.Environ(),
|
||||||
|
"GIT_CONFIG_GLOBAL=/dev/null", "GIT_CONFIG_SYSTEM=/dev/null",
|
||||||
|
"GIT_AUTHOR_DATE=@"+epoch+" +0000",
|
||||||
|
"GIT_COMMITTER_DATE=@"+epoch+" +0000",
|
||||||
|
)
|
||||||
|
if out, err := cmd.CombinedOutput(); err != nil {
|
||||||
|
t.Fatalf("git %v: %v\n%s", args, err, out)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// gitOutput runs a git command in dir and returns its combined output.
|
||||||
|
func gitOutput(t *testing.T, dir string, args ...string) string {
|
||||||
|
t.Helper()
|
||||||
|
cmd := exec.Command("git", args...)
|
||||||
|
cmd.Dir = dir
|
||||||
|
cmd.Env = append(os.Environ(), "GIT_CONFIG_GLOBAL=/dev/null", "GIT_CONFIG_SYSTEM=/dev/null")
|
||||||
|
out, err := cmd.CombinedOutput()
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("git %v: %v\n%s", args, err, out)
|
||||||
|
}
|
||||||
|
return string(out)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Tests do not ship, so a test-only commit must not re-version an artifact.
|
||||||
|
//
|
||||||
|
// `go build` drops *_test.go outright and the Vite build never imports a
|
||||||
|
// .test.ts, so neither reaches an image or an APK. This repo has no tests/
|
||||||
|
// tree — Go tests sit inline beside the code — so the exclusions are globs,
|
||||||
|
// and a glob is easy to get subtly wrong in a way that still looks right.
|
||||||
|
func TestVersionName_TestsDoNotReVersionTheArtifact(t *testing.T) {
|
||||||
|
const (
|
||||||
|
shipped = "1757443736" // 2025.09.09.1848
|
||||||
|
later = "1789000920" // 2026.09.10.0042
|
||||||
|
)
|
||||||
|
for _, tc := range []struct{ name, path string }{
|
||||||
|
{"go test beside its source", "internal/server/thing_test.go"},
|
||||||
|
{"web unit test", "web/src/lib/api/admin.test.ts"},
|
||||||
|
{"web script test", "web/scripts/tokens-to-css.test.js"},
|
||||||
|
{"android JVM unit test", "android/app/src/test/java/A.kt"},
|
||||||
|
{"vitest harness config", "web/vitest.config.ts"},
|
||||||
|
} {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
dir := gitRepo(t)
|
||||||
|
commitFile(t, dir, "internal/server/thing.go", shipped)
|
||||||
|
commitFile(t, dir, tc.path, later)
|
||||||
|
|
||||||
|
out, err := versionIn(t, dir)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("version.sh failed: %v\n%s", err, out)
|
||||||
|
}
|
||||||
|
if !strings.Contains(out, "name=2025.09.09.1848") {
|
||||||
|
t.Errorf("a commit touching only %s moved the version; tests do not ship\n%s", tc.path, out)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// The direction that actually costs something, and the reason the exclusions
|
||||||
|
// above are globs over FILES rather than over their directories.
|
||||||
|
//
|
||||||
|
// `':!internal'` would satisfy every assertion in the test above while
|
||||||
|
// silently excluding the entire server. This pins the opposite: a commit that
|
||||||
|
// changes a test AND the source under it must still move the version, because
|
||||||
|
// the source path matches on its own. Without this, a too-broad exclusion
|
||||||
|
// reads as a passing test suite and ships an artifact under a stale version.
|
||||||
|
func TestVersionName_ATestAlongsideItsSourceStillCounts(t *testing.T) {
|
||||||
|
const (
|
||||||
|
shipped = "1757443736"
|
||||||
|
later = "1789000920"
|
||||||
|
)
|
||||||
|
dir := gitRepo(t)
|
||||||
|
commitFile(t, dir, "internal/server/thing.go", shipped)
|
||||||
|
commitFiles(t, dir, later,
|
||||||
|
"internal/server/thing.go",
|
||||||
|
"internal/server/thing_test.go",
|
||||||
|
)
|
||||||
|
|
||||||
|
// The commit must actually contain BOTH paths. Rewriting a file with
|
||||||
|
// identical bytes records nothing, and this assertion would then be
|
||||||
|
// checking a test-only commit while appearing to check a mixed one.
|
||||||
|
touched := gitOutput(t, dir, "show", "--name-only", "--format=", "HEAD")
|
||||||
|
for _, want := range []string{"internal/server/thing.go", "internal/server/thing_test.go"} {
|
||||||
|
if !strings.Contains(touched, want) {
|
||||||
|
t.Fatalf("fixture is wrong: HEAD does not contain %s\n%s", want, touched)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
out, err := versionIn(t, dir)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("version.sh failed: %v\n%s", err, out)
|
||||||
|
}
|
||||||
|
if !strings.Contains(out, "name=2026.09.10.0042") {
|
||||||
|
t.Errorf("a source change accompanied by a test change did NOT move the version — "+
|
||||||
|
"the exclusions are matching directories rather than test files\n%s", out)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -218,6 +218,7 @@ func (s *Server) handleHealthz(w http.ResponseWriter, _ *http.Request) {
|
|||||||
_ = json.NewEncoder(w).Encode(map[string]string{
|
_ = json.NewEncoder(w).Encode(map[string]string{
|
||||||
"status": "ok",
|
"status": "ok",
|
||||||
"version": ServerVersion,
|
"version": ServerVersion,
|
||||||
|
"channel": ServerChannel,
|
||||||
"min_client_version": MinClientVersion,
|
"min_client_version": MinClientVersion,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -5,12 +5,27 @@ package server
|
|||||||
// older clients see version_too_old at /healthz and refuse to operate.
|
// older clients see version_too_old at /healthz and refuse to operate.
|
||||||
const MinClientVersion = "0.1.0"
|
const MinClientVersion = "0.1.0"
|
||||||
|
|
||||||
// ServerVersion is the deployed server image's version tag. Defaults to
|
// ServerVersion is the build's own version name — YYYY.MM.DD.HHMM, derived
|
||||||
// "dev" for local builds; overridden at link time via:
|
// from the commit it was built from by ci/version.sh. Defaults to "dev" for
|
||||||
|
// local builds; overridden at link time via:
|
||||||
//
|
//
|
||||||
// -ldflags="-X 'git.fabledsword.com/bvandeusen/minstrel/internal/server.ServerVersion=v2026.05.10.2'"
|
// -ldflags="-X 'git.fabledsword.com/bvandeusen/minstrel/internal/server.ServerVersion=2026.09.10.1449'"
|
||||||
//
|
//
|
||||||
// release.yml passes the git tag through MINSTREL_VERSION build-arg →
|
// release.yml passes it through the MINSTREL_VERSION build-arg → Dockerfile
|
||||||
// Dockerfile ldflag. Surfaced at /healthz so operators can verify which
|
// ldflag. Surfaced at /healthz so operators can verify which image their
|
||||||
// image their container is running without exec'ing into it.
|
// container is running without exec'ing into it.
|
||||||
|
//
|
||||||
|
// This carried the literal strings "main" and "dev" until 2026-09-10, which
|
||||||
|
// made every image on a channel report the same thing forever. It stopped
|
||||||
|
// being cosmetic when :vYYYY.MM.DD.HHMM image tags were retired (family rule
|
||||||
|
// 145): this is now the ONLY thing that says which build is running.
|
||||||
var ServerVersion = "dev"
|
var ServerVersion = "dev"
|
||||||
|
|
||||||
|
// ServerChannel is which line this build came off — "stable" or "dev", or
|
||||||
|
// "local" for a plain `docker build`.
|
||||||
|
//
|
||||||
|
// A SIBLING FIELD, never a suffix inside ServerVersion (family rule 149). The
|
||||||
|
// same commit built on both lanes reports the same version and differs only
|
||||||
|
// here; folding the two together is what makes a version string stop being
|
||||||
|
// comparable.
|
||||||
|
var ServerChannel = "local"
|
||||||
|
|||||||
@@ -34,14 +34,17 @@ export type DiscoverTuning = {
|
|||||||
snooze_days: number;
|
snooze_days: number;
|
||||||
};
|
};
|
||||||
|
|
||||||
export type TuningScope = 'radio' | 'daily_mix' | 'taste' | 'discover';
|
export type TuningScope = 'radio' | 'daily_mix' | 'songs_like' | 'taste' | 'discover';
|
||||||
|
|
||||||
|
/** The weight-profile scopes, as distinct from the singleton-settings scopes. */
|
||||||
|
export type WeightProfileScope = 'radio' | 'daily_mix' | 'songs_like';
|
||||||
|
|
||||||
export type TuningSnapshot = {
|
export type TuningSnapshot = {
|
||||||
profiles: Record<'radio' | 'daily_mix', WeightProfile>;
|
profiles: Record<WeightProfileScope, WeightProfile>;
|
||||||
taste: TasteTuning;
|
taste: TasteTuning;
|
||||||
discover: DiscoverTuning;
|
discover: DiscoverTuning;
|
||||||
shipped: {
|
shipped: {
|
||||||
profiles: Record<'radio' | 'daily_mix', WeightProfile>;
|
profiles: Record<WeightProfileScope, WeightProfile>;
|
||||||
taste: TasteTuning;
|
taste: TasteTuning;
|
||||||
discover: DiscoverTuning;
|
discover: DiscoverTuning;
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -6,26 +6,36 @@
|
|||||||
// is actually running (came up debugging the in-app update flow when
|
// is actually running (came up debugging the in-app update flow when
|
||||||
// it wasn't obvious whether v2026.05.10.0 or .1 was deployed).
|
// it wasn't obvious whether v2026.05.10.0 or .1 was deployed).
|
||||||
//
|
//
|
||||||
|
// This is now the ONLY place an operator can see which build they are on.
|
||||||
|
// Image tags stopped carrying the version on 2026-09-10 — :latest and :dev
|
||||||
|
// are rolling names and :<sha> answers "which commit", not "which build" —
|
||||||
|
// so the server's self-report is the answer.
|
||||||
|
//
|
||||||
|
// The channel is shown BESIDE the version, never spliced into it: the same
|
||||||
|
// commit built on both lanes reports an identical version and differs only
|
||||||
|
// in channel, so "2026.09.10.1449 · dev" and "2026.09.10.1449 · stable" are
|
||||||
|
// the same code on two lines. Suppressed for stable, which is the
|
||||||
|
// unremarkable case and would just be noise on every install.
|
||||||
|
//
|
||||||
// /healthz is unauthenticated, so the bare fetch works without
|
// /healthz is unauthenticated, so the bare fetch works without
|
||||||
// credentials. Renders nothing on parse failure or pre-version
|
// credentials. Renders nothing on parse failure or pre-version
|
||||||
// images that don't include the field — graceful degradation.
|
// images that don't include the field — graceful degradation.
|
||||||
|
|
||||||
type Health = { status: string; version?: string };
|
type Health = { status: string; version?: string; channel?: string };
|
||||||
|
|
||||||
let version = $state<string | null>(null);
|
let version = $state<string | null>(null);
|
||||||
|
let channel = $state<string | null>(null);
|
||||||
|
|
||||||
onMount(async () => {
|
onMount(async () => {
|
||||||
try {
|
try {
|
||||||
const res = await fetch('/healthz');
|
const res = await fetch('/healthz');
|
||||||
if (!res.ok) return;
|
if (!res.ok) return;
|
||||||
const body = (await res.json()) as Partial<Health>;
|
const body = (await res.json()) as Partial<Health>;
|
||||||
if (body.version && body.version !== 'dev') {
|
if (!body.version) return;
|
||||||
version = body.version;
|
version = body.version;
|
||||||
} else if (body.version === 'dev') {
|
// Reported verbatim rather than validated against an enum — a build
|
||||||
// Local dev images report "dev" — show it so the operator
|
// claiming something unexpected is better shown than dropped.
|
||||||
// can tell they're not on a release tag.
|
channel = body.channel && body.channel !== 'stable' ? body.channel : null;
|
||||||
version = 'dev';
|
|
||||||
}
|
|
||||||
} catch {
|
} catch {
|
||||||
// network / parse error — silent.
|
// network / parse error — silent.
|
||||||
}
|
}
|
||||||
@@ -33,5 +43,7 @@
|
|||||||
</script>
|
</script>
|
||||||
|
|
||||||
{#if version}
|
{#if version}
|
||||||
<p class="text-xs text-text-secondary">Server {version}</p>
|
<p class="text-xs text-text-secondary">
|
||||||
|
Server {version}{#if channel} · {channel}{/if}
|
||||||
|
</p>
|
||||||
{/if}
|
{/if}
|
||||||
|
|||||||
@@ -6,6 +6,7 @@
|
|||||||
resetTuning,
|
resetTuning,
|
||||||
getTrends,
|
getTrends,
|
||||||
type TuningScope,
|
type TuningScope,
|
||||||
|
type WeightProfileScope,
|
||||||
type TuningSnapshot,
|
type TuningSnapshot,
|
||||||
type WeightProfile,
|
type WeightProfile,
|
||||||
type TasteTuning,
|
type TasteTuning,
|
||||||
@@ -49,9 +50,15 @@
|
|||||||
{ key: 'snooze_days', label: 'Snooze length (days)', hint: 'How long "not right now" parks a suggestion before it returns on its own. Records no opinion about the artist and never feeds the taste profile.' }
|
{ key: 'snooze_days', label: 'Snooze length (days)', hint: 'How long "not right now" parks a suggestion before it returns on its own. Records no opinion about the artist and never feeds the taste profile.' }
|
||||||
];
|
];
|
||||||
|
|
||||||
const profileScopes: { scope: 'radio' | 'daily_mix'; label: string; blurb: string }[] = [
|
const profileScopes: { scope: WeightProfileScope; label: string; blurb: string }[] = [
|
||||||
{ scope: 'radio', label: 'Radio', blurb: 'Seed-directed listening — the user picked a direction.' },
|
{ scope: 'radio', label: 'Radio', blurb: 'Seed-directed listening — the user picked a direction.' },
|
||||||
{ scope: 'daily_mix', label: 'Daily mixes', blurb: 'For You, Songs like…, and the discovery mixes.' }
|
{ scope: 'daily_mix', label: 'Daily mixes', blurb: 'For You, the discovery mixes, and "You might like".' },
|
||||||
|
{
|
||||||
|
scope: 'songs_like',
|
||||||
|
label: 'Songs like…',
|
||||||
|
blurb:
|
||||||
|
'The tightest surface: everything here should sound like the seed track. Similarity dominates on purpose — raising like/taste/recency here is what makes these mixes wander.'
|
||||||
|
}
|
||||||
];
|
];
|
||||||
|
|
||||||
let snapshot = $state<TuningSnapshot | null>(null);
|
let snapshot = $state<TuningSnapshot | null>(null);
|
||||||
@@ -64,10 +71,11 @@
|
|||||||
const f: Record<string, Record<string, string>> = {
|
const f: Record<string, Record<string, string>> = {
|
||||||
radio: {},
|
radio: {},
|
||||||
daily_mix: {},
|
daily_mix: {},
|
||||||
|
songs_like: {},
|
||||||
taste: {},
|
taste: {},
|
||||||
discover: {}
|
discover: {}
|
||||||
};
|
};
|
||||||
for (const p of ['radio', 'daily_mix'] as const) {
|
for (const p of profileScopes.map((s) => s.scope)) {
|
||||||
for (const { key } of weightFields) f[p][key] = String(snap.profiles[p][key]);
|
for (const { key } of weightFields) f[p][key] = String(snap.profiles[p][key]);
|
||||||
}
|
}
|
||||||
for (const { key } of tasteFields) f.taste[key] = String(snap.taste[key]);
|
for (const { key } of tasteFields) f.taste[key] = String(snap.taste[key]);
|
||||||
|
|||||||
@@ -55,11 +55,19 @@ const discover = (over: Partial<Record<string, number>> = {}) => ({
|
|||||||
// new scopes to BOTH this fixture and `shipped`.
|
// new scopes to BOTH this fixture and `shipped`.
|
||||||
function snapshot(over: Partial<TuningSnapshot> = {}): TuningSnapshot {
|
function snapshot(over: Partial<TuningSnapshot> = {}): TuningSnapshot {
|
||||||
return {
|
return {
|
||||||
profiles: { radio: weights({ taste_weight: 1 }), daily_mix: weights() },
|
profiles: {
|
||||||
|
radio: weights({ taste_weight: 1 }),
|
||||||
|
daily_mix: weights(),
|
||||||
|
songs_like: weights({ similarity_weight: 4, like_boost: 0.5, taste_weight: 0.25 })
|
||||||
|
},
|
||||||
taste: taste(),
|
taste: taste(),
|
||||||
discover: discover(),
|
discover: discover(),
|
||||||
shipped: {
|
shipped: {
|
||||||
profiles: { radio: weights({ taste_weight: 1 }), daily_mix: weights() },
|
profiles: {
|
||||||
|
radio: weights({ taste_weight: 1 }),
|
||||||
|
daily_mix: weights(),
|
||||||
|
songs_like: weights({ similarity_weight: 4, like_boost: 0.5, taste_weight: 0.25 })
|
||||||
|
},
|
||||||
taste: taste(),
|
taste: taste(),
|
||||||
discover: discover()
|
discover: discover()
|
||||||
},
|
},
|
||||||
@@ -75,11 +83,12 @@ beforeEach(() => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
describe('Admin tuning page', () => {
|
describe('Admin tuning page', () => {
|
||||||
test('renders both profiles and the taste card with current values', async () => {
|
test('renders every weight profile and the taste card with current values', async () => {
|
||||||
(getTuning as ReturnType<typeof vi.fn>).mockResolvedValue(snapshot());
|
(getTuning as ReturnType<typeof vi.fn>).mockResolvedValue(snapshot());
|
||||||
render(TuningPage);
|
render(TuningPage);
|
||||||
await waitFor(() => expect(screen.getByText('Radio')).toBeInTheDocument());
|
await waitFor(() => expect(screen.getByText('Radio')).toBeInTheDocument());
|
||||||
expect(screen.getByText('Daily mixes')).toBeInTheDocument();
|
expect(screen.getByText('Daily mixes')).toBeInTheDocument();
|
||||||
|
expect(screen.getByText('Songs like…')).toBeInTheDocument();
|
||||||
expect(screen.getByText('Taste profile build')).toBeInTheDocument();
|
expect(screen.getByText('Taste profile build')).toBeInTheDocument();
|
||||||
const radioTaste = screen.getByLabelText(/taste weight/i, {
|
const radioTaste = screen.getByLabelText(/taste weight/i, {
|
||||||
selector: '#radio-taste_weight'
|
selector: '#radio-taste_weight'
|
||||||
@@ -89,6 +98,27 @@ describe('Admin tuning page', () => {
|
|||||||
expect(halfLife.value).toBe('75');
|
expect(halfLife.value).toBe('75');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Songs-like is a separate weight profile precisely so it can be tuned
|
||||||
|
// apart from For-You (#3881). If its card stopped rendering its OWN values
|
||||||
|
// — or quietly fell back to daily_mix's — the split would exist in the
|
||||||
|
// backend and be unreachable in the UI, which is the same as not shipping
|
||||||
|
// it (rule 27).
|
||||||
|
test('the songs-like card carries its own weights, not daily_mix\'s', async () => {
|
||||||
|
(getTuning as ReturnType<typeof vi.fn>).mockResolvedValue(snapshot());
|
||||||
|
render(TuningPage);
|
||||||
|
await waitFor(() => expect(screen.getByText('Songs like…')).toBeInTheDocument());
|
||||||
|
|
||||||
|
const sim = document.getElementById('songs_like-similarity_weight') as HTMLInputElement;
|
||||||
|
const like = document.getElementById('songs_like-like_boost') as HTMLInputElement;
|
||||||
|
expect(sim.value).toBe('4');
|
||||||
|
expect(like.value).toBe('0.5');
|
||||||
|
|
||||||
|
// The contrast that makes the card worth having: daily_mix must still show
|
||||||
|
// its own, different numbers on the same page.
|
||||||
|
const dailySim = document.getElementById('daily_mix-similarity_weight') as HTMLInputElement;
|
||||||
|
expect(dailySim.value).not.toBe(sim.value);
|
||||||
|
});
|
||||||
|
|
||||||
test('save sends only the changed fields for the scope', async () => {
|
test('save sends only the changed fields for the scope', async () => {
|
||||||
(getTuning as ReturnType<typeof vi.fn>).mockResolvedValue(snapshot());
|
(getTuning as ReturnType<typeof vi.fn>).mockResolvedValue(snapshot());
|
||||||
(patchTuning as ReturnType<typeof vi.fn>).mockResolvedValue(snapshot());
|
(patchTuning as ReturnType<typeof vi.fn>).mockResolvedValue(snapshot());
|
||||||
|
|||||||
Reference in New Issue
Block a user