From a5bb79f2a8ad6e105b568087ec83745a8567382c Mon Sep 17 00:00:00 2001
From: Bryan Van Deusen <bryan.vandeusen@fabledsword.com>
Date: Wed, 29 Apr 2026 15:19:54 -0400
Subject: [PATCH] fix(db): clarify lidarr_requests query semantics

- HasNonTerminalRequestForMBID: use named parameter @mbid so the
  generated Go signature is `mbid string` instead of the misleading
  `lidarrArtistMbid string` (the value applies to all three MBID
  columns, not just the artist column)
- CancelLidarrRequest: comment dual role of $2 (ownership guard +
  decided_by audit field) for future readers
- CompleteLidarrRequest: document the per-kind one-of-three contract
  for matched_*_id parameters

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 internal/db/dbq/lidarr_requests.sql.go  | 18 ++++++++++++++----
 internal/db/queries/lidarr_requests.sql | 20 +++++++++++++++-----
 2 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/internal/db/dbq/lidarr_requests.sql.go b/internal/db/dbq/lidarr_requests.sql.go
index 70676607..ac9d796e 100644
--- a/internal/db/dbq/lidarr_requests.sql.go
+++ b/internal/db/dbq/lidarr_requests.sql.go
@@ -80,6 +80,11 @@ type CancelLidarrRequestParams struct {
 	DecidedBy pgtype.UUID
 }
 
+// $2 serves as both the ownership guard (user_id = $2) and the
+// decided_by audit field. The generated parameter struct names it
+// DecidedBy — callers must pass the cancelling user's UUID, and
+// a wrong UUID will silently no-op (zero rows updated) rather than
+// erroring at the SQL layer.
 func (q *Queries) CancelLidarrRequest(ctx context.Context, arg CancelLidarrRequestParams) (LidarrRequest, error) {
 	row := q.db.QueryRow(ctx, cancelLidarrRequest, arg.ID, arg.DecidedBy)
 	var i LidarrRequest
@@ -129,7 +134,11 @@ type CompleteLidarrRequestParams struct {
 }
 
 // Reconciler transitions an approved request to completed when its
-// target track/album/artist has appeared in the library.
+// target track/album/artist has appeared in the library. Callers
+// should set ONLY the matched_*_id corresponding to the request's
+// kind and pass pgtype.UUID{} (Valid: false) for the others —
+// per-kind one-of-three is the contract, NULL for the others
+// preserves auditability if the matched row is later evicted.
 func (q *Queries) CompleteLidarrRequest(ctx context.Context, arg CompleteLidarrRequestParams) (LidarrRequest, error) {
 	row := q.db.QueryRow(ctx, completeLidarrRequest,
 		arg.ID,
@@ -268,9 +277,10 @@ SELECT EXISTS (
 // Returns true if any user has a pending/approved/completed request
 // whose MBID matches at the given level. Used to set the `requested`
 // flag on /api/lidarr/search responses. Terminal-status (rejected,
-// failed) rows do not count.
-func (q *Queries) HasNonTerminalRequestForMBID(ctx context.Context, lidarrArtistMbid string) (bool, error) {
-	row := q.db.QueryRow(ctx, hasNonTerminalRequestForMBID, lidarrArtistMbid)
+// failed) rows do not count. The same MBID parameter is checked
+// against artist/album/track columns based on kind.
+func (q *Queries) HasNonTerminalRequestForMBID(ctx context.Context, mbid string) (bool, error) {
+	row := q.db.QueryRow(ctx, hasNonTerminalRequestForMBID, mbid)
 	var exists bool
 	err := row.Scan(&exists)
 	return exists, err
diff --git a/internal/db/queries/lidarr_requests.sql b/internal/db/queries/lidarr_requests.sql
index 9641581b..d075a07a 100644
--- a/internal/db/queries/lidarr_requests.sql
+++ b/internal/db/queries/lidarr_requests.sql
@@ -49,6 +49,11 @@ UPDATE lidarr_requests
  RETURNING *;
 
 -- name: CancelLidarrRequest :one
+-- $2 serves as both the ownership guard (user_id = $2) and the
+-- decided_by audit field. The generated parameter struct names it
+-- DecidedBy — callers must pass the cancelling user's UUID, and
+-- a wrong UUID will silently no-op (zero rows updated) rather than
+-- erroring at the SQL layer.
 UPDATE lidarr_requests
    SET status     = 'rejected',
        notes      = 'cancelled by user',
@@ -60,7 +65,11 @@ UPDATE lidarr_requests
 
 -- name: CompleteLidarrRequest :one
 -- Reconciler transitions an approved request to completed when its
--- target track/album/artist has appeared in the library.
+-- target track/album/artist has appeared in the library. Callers
+-- should set ONLY the matched_*_id corresponding to the request's
+-- kind and pass pgtype.UUID{} (Valid: false) for the others —
+-- per-kind one-of-three is the contract, NULL for the others
+-- preserves auditability if the matched row is later evicted.
 UPDATE lidarr_requests
    SET status            = 'completed',
        matched_track_id  = $2,
@@ -75,11 +84,12 @@ UPDATE lidarr_requests
 -- Returns true if any user has a pending/approved/completed request
 -- whose MBID matches at the given level. Used to set the `requested`
 -- flag on /api/lidarr/search responses. Terminal-status (rejected,
--- failed) rows do not count.
+-- failed) rows do not count. The same MBID parameter is checked
+-- against artist/album/track columns based on kind.
 SELECT EXISTS (
     SELECT 1 FROM lidarr_requests
     WHERE status IN ('pending', 'approved', 'completed')
-      AND ((kind = 'artist' AND lidarr_artist_mbid = $1)
-        OR (kind = 'album'  AND lidarr_album_mbid  = $1)
-        OR (kind = 'track'  AND lidarr_track_mbid  = $1))
+      AND ((kind = 'artist' AND lidarr_artist_mbid = @mbid)
+        OR (kind = 'album'  AND lidarr_album_mbid  = @mbid)
+        OR (kind = 'track'  AND lidarr_track_mbid  = @mbid))
 );