Skip to content

feat: catalog_valuations — persisted valuation history (chat#1889 row 15) #52

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sweetmantech merged 1 commit into from
Jul 29, 2026
Merged

feat: catalog_valuations — persisted valuation history (chat#1889 row 15) #52

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions supabase/migrations/20260729230000_create_catalog_valuations.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
-- Create catalog_valuations: persisted valuation history per catalog
-- (recoupable/chat#1889 row 15 — the keystone of the 2026-07-29 reorder).
--
-- The estimated catalog value is currently recomputed live on every read
-- (GET /api/catalogs/{id}/measurements derives the band at read time) and
-- stored nowhere. That costs the product its retention hook — a weekly report
-- can never say "your number moved" without two numbers to compare — and costs
-- sales a sortable Attio Catalog Value field (today hand-priced per lead).
--
-- One row per band computation, at most one per catalog per day from the
-- read path (valuation runs always insert). History is the point: columns on
-- catalogs would only ever hold the latest value.
--
-- Written by api: runValuationHandler (funnel + onboarding seeding) and the
-- measurements read path (daily-deduped). Read by
-- GET /api/catalogs/{catalogId}/valuations (contract: docs#279).
--
-- FK to catalogs with ON DELETE CASCADE: a valuation row is meaningless
-- without its catalog, unlike the loose-id bookkeeping tables.

CREATE TABLE IF NOT EXISTS public.catalog_valuations (
id UUID PRIMARY KEY DEFAULT gen_random_uuid(),
catalog_id UUID NOT NULL REFERENCES public.catalogs(id) ON DELETE CASCADE,
low NUMERIC NOT NULL, -- band low, USD
mid NUMERIC NOT NULL, -- band midpoint, USD
high NUMERIC NOT NULL, -- band high, USD
measured_song_count INTEGER NOT NULL, -- songs measured in the underlying capture
total_streams BIGINT NOT NULL, -- whole-catalog lifetime streams at measurement
measured_at TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT now(),
created_at TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT now()

@cubic-dev-ai cubic-dev-ai Bot Jul 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The table only enforces NOT NULL on low/mid/high, measured_song_count, and total_streams. Nothing stops a writer from inserting negative counts/streams or an invalid band where low > mid or mid > high. Consider adding CHECK constraints (e.g. CHECK (low >= 0 AND low <= mid AND mid <= high), CHECK (measured_song_count >= 0), CHECK (total_streams >= 0)) so the schema itself preserves the valuation history contract.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/migrations/20260729230000_create_catalog_valuations.sql, line 30:

<comment>The table only enforces NOT NULL on low/mid/high, measured_song_count, and total_streams. Nothing stops a writer from inserting negative counts/streams or an invalid band where low > mid or mid > high. Consider adding CHECK constraints (e.g. CHECK (low >= 0 AND low <= mid AND mid <= high), CHECK (measured_song_count >= 0), CHECK (total_streams >= 0)) so the schema itself preserves the valuation history contract.</comment>

<file context>
@@ -0,0 +1,35 @@
+    measured_song_count INTEGER  NOT NULL,  -- songs measured in the underlying capture
+    total_streams       BIGINT   NOT NULL,  -- whole-catalog lifetime streams at measurement
+    measured_at         TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT now(),
+    created_at          TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT now()
+);
+
</file context>

);
Comment on lines +21 to +31

@coderabbitai coderabbitai Bot Jul 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Enforce valuation invariants in the schema.

NOT NULL still permits negative counts/streams and invalid bands such as low > mid or mid > high. Add table-level CHECK constraints so every writer preserves the history contract.

Proposed fix 
     measured_at         TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT now(),
-    created_at          TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT now()
+    created_at          TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT now(),
+    CONSTRAINT catalog_valuations_band_order_chk
+      CHECK (low >= 0 AND low <= mid AND mid <= high),
+    CONSTRAINT catalog_valuations_song_count_chk
+      CHECK (measured_song_count >= 0),
+    CONSTRAINT catalog_valuations_streams_chk
+      CHECK (total_streams >= 0)
 );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
CREATE TABLE IF NOT EXISTS public.catalog_valuations (
id UUID PRIMARY KEY DEFAULT gen_random_uuid(),
catalog_id UUID NOT NULL REFERENCES public.catalogs(id) ON DELETE CASCADE,
low NUMERIC NOT NULL, -- band low, USD
mid NUMERIC NOT NULL, -- band midpoint, USD
high NUMERIC NOT NULL, -- band high, USD
measured_song_count INTEGER NOT NULL, -- songs measured in the underlying capture
total_streams BIGINT NOT NULL, -- whole-catalog lifetime streams at measurement
measured_at TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT now(),
created_at TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT now()
);
CREATE TABLE IF NOT EXISTS public.catalog_valuations (
id UUID PRIMARY KEY DEFAULT gen_random_uuid(),
catalog_id UUID NOT NULL REFERENCES public.catalogs(id) ON DELETE CASCADE,
low NUMERIC NOT NULL, -- band low, USD
mid NUMERIC NOT NULL, -- band midpoint, USD
high NUMERIC NOT NULL, -- band high, USD
measured_song_count INTEGER NOT NULL, -- songs measured in the underlying capture
total_streams BIGINT NOT NULL, -- whole-catalog lifetime streams at measurement
measured_at TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT now(),
created_at TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT now(),
CONSTRAINT catalog_valuations_band_order_chk
CHECK (low >= 0 AND low <= mid AND mid <= high),
CONSTRAINT catalog_valuations_song_count_chk
CHECK (measured_song_count >= 0),
CONSTRAINT catalog_valuations_streams_chk
CHECK (total_streams >= 0)
);
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@supabase/migrations/20260729230000_create_catalog_valuations.sql` around
lines 21 - 31, Add table-level CHECK constraints to public.catalog_valuations
ensuring low, mid, and high are non-negative and ordered low <= mid <= high, and
measured_song_count and total_streams are non-negative. Keep the existing
columns and defaults unchanged.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "## migration file excerpt"
if [ -f supabase/migrations/20260729230000_create_catalog_valuations.sql ]; then
  cat -n supabase/migrations/20260729230000_create_catalog_valuations.sql
else
  echo "migration file not found"
fi

echo
echo "## related catalog policy/RLS references"
rg -n "catalog_valuations|catalogs|RLS|enable row level security|ALTER TABLE .* ENABLE ROW LEVEL SECURITY|CREATE POLICY|GRANT .* catalog_valuations|pg_ls_rel_acl|supabase/functions|auth/user" -S supabase . -g '!node_modules' -g '!dist' -g '!build' | head -200

echo
echo "## SQL policy creation / grants candidates"
fd -e sql . supabase public supabase -x sh -c 'echo "--- $1"; rg -n "catalog_valuations|catalogs|ENABLE ROW LEVEL SECURITY|ALTER TABLE .* ENABLE ROW LEVEL SECURITY|CREATE POLICY|GRANT .* catalog|revoke all|role anon|role authenticated" "{}" -S' sh {}

echo
echo "## git diff stat/name/status"
git diff --stat || true
git diff --name-only || true

Repository: recoupable/database

Length of output: 50375

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "## catalogs migration"
cat -n supabase/migrations/20251005212508_create_catalogs_table.sql

echo
echo "## catalog-songs/measurements related migrations excerpts"
cat -n supabase/migrations/20260707120000_catalog_measurements_read_functions.sql
echo "---"
if [ -f supabase/migrations/20251005214038_create_catalog_songs_table.sql ]; then cat -n supabase/migrations/20251005214038_create_catalog_songs_table.sql; fi
echo "---"
if [ -f supabase/migrations/20251005210048_create_songs_table.sql ]; then sed -n '1,80p' supabase/migrations/20251005210048_create_songs_table.sql | cat -n; fi

echo
echo "## valuations route/API references"
rg -n "valuations|/catalogs/.*/valuations|catalogValuations|getCatalogValuations|runValuationHandler|catalog_id|catalogValuation" -S . \
  -g '!node_modules' -g '!dist' -g '!build' | head -250

echo
echo "## table grants and RLS statements for public catalog tables (targeted)"
python3 - <<'PY'
from pathlib import Path
needle = ['catalogs', 'catalog_songs', 'catalog_valuations', 'account_catalogs']
for p in Path('supabase/migrations').glob('*.sql'):
    s = p.read_text(errors='ignore')
    hits = [key for key in needle if key in s.lower()]
    if not hits:
        continue
    stmts = []
    for line in s.splitlines():
        l=line.lower().strip()
        if any(key in l for key in hits):
            stmts.append(line)
    print(f'--- {p}')
    for i, line in enumerate(s.splitlines(), 1):
        if any(key in line.lower() for key in hits) or any(x in l_for_line for x in ['grant ', 'alter table', 'create policy', 'enable row level security', 'revoke ']):
            print(f'{i:04d}: {line}')
    print()
PY

Repository: recoupable/database

Length of output: 11784

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "## account_catalogs migration excerpt"
sed -n '1,120p' supabase/migrations/20251005214926_create_account_catalogs_table.sql | cat -n

echo
echo "## valuations references across repo"
rg -n "catalog_valuation|catalogValuation|/valuations|valuations" -S . \
  -g '!node_modules' -g '!dist' -g '!build' -g '!supabase/migrations/20260729230000_create_catalog_valuations.sql' | head -200

echo
echo "## all catalog_valuations references"
rg -n "catalog_valuations|catalog_valuation" -S . \
  -g '!node_modules' -g '!dist' -g '!build' | head -250

Repository: recoupable/database

Length of output: 3595

Grant catalog_valuations to the service-role endpoint or add RLS policies for authenticated access.

This table is documented as read by GET /api/catalogs/{catalogId}/valuations, but unlike the neighbor catalog tables, it has no grants/RLS. As written, the service-role writes can run and PostgREST queries will fail with the “new row violates row-level security policy” check on access-controlled tables, so add the intended grant/policy path here.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@supabase/migrations/20260729230000_create_catalog_valuations.sql` around
lines 21 - 31, Update the catalog_valuations table definition to establish its
intended API access path, matching the neighboring catalog tables: either grant
the required privileges to the service-role endpoint or enable RLS and add
policies permitting authenticated reads for GET
/api/catalogs/{catalogId}/valuations. Ensure service-role writes remain
permitted and the catalog_id scope is enforced consistently with existing
policies.


-- The read is always "latest rows for one catalog".
CREATE INDEX IF NOT EXISTS catalog_valuations_catalog_measured_idx
ON public.catalog_valuations (catalog_id, measured_at DESC);

@cubic-dev-ai cubic-dev-ai Bot Jul 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Catalog valuation history can be read or modified through the public data API without catalog-level authorization. Enable RLS here (the API's service role still bypasses it), matching catalogs and the measurement store.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/migrations/20260729230000_create_catalog_valuations.sql, line 35:

<comment>Catalog valuation history can be read or modified through the public data API without catalog-level authorization. Enable RLS here (the API's service role still bypasses it), matching `catalogs` and the measurement store.</comment>

<file context>
@@ -0,0 +1,35 @@
+
+-- The read is always "latest rows for one catalog".
+CREATE INDEX IF NOT EXISTS catalog_valuations_catalog_measured_idx
+  ON public.catalog_valuations (catalog_id, measured_at DESC);
</file context>