From f6bce9ecd6789561cd933a9e969e28b242966da4 Mon Sep 17 00:00:00 2001 From: JOY <5027251+JOY@users.noreply.github.com> Date: Sat, 31 Oct 2026 20:41:42 +0700 Subject: [PATCH 1/5] docs(proposals): dos-me migration for criteria ratings, pros and cons (p2) Phase P2 of #1531 (D2, D3, D9, D10), for JOY to review and apply through DOS-Me; nothing is applied from DOS/Rate. New review columns (exact star, verdict, criteria version, value score), rate.review_points and rate.product_points with RLS, and the master columns rating_score (Bayesian, k 3, mu 4.2), rating_weight_sum and criteria_scores (D9 thresholds). The live refresh_master_product_rating is renamed to a base and wrapped, so every caller also runs the new extras function. Verify and rollback files, and a Docker replay in CI that applies it on the reference stack and after M0 and M1+M2. Refs #1543 Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 3 + ...10-31-dos-me-criteria-ratings-rollback.sql | 42 +++ ...6-10-31-dos-me-criteria-ratings-verify.sql | 64 +++++ .../2026-10-31-dos-me-criteria-ratings.md | 57 ++++ .../2026-10-31-dos-me-criteria-ratings.sql | 251 ++++++++++++++++++ scripts/test-criteria-ratings-proposal.mjs | 209 +++++++++++++++ 6 files changed, 626 insertions(+) create mode 100644 Docs/Proposals/2026-10-31-dos-me-criteria-ratings-rollback.sql create mode 100644 Docs/Proposals/2026-10-31-dos-me-criteria-ratings-verify.sql create mode 100644 Docs/Proposals/2026-10-31-dos-me-criteria-ratings.md create mode 100644 Docs/Proposals/2026-10-31-dos-me-criteria-ratings.sql create mode 100644 scripts/test-criteria-ratings-proposal.mjs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e8a171cae..81ceff8a7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -73,6 +73,9 @@ jobs: - name: Replay the DOS-Me Item RPC and M3/M4 proposals (Docker) run: node scripts/test-m3-m4-item-rpcs-proposal.mjs + - name: Replay the DOS-Me criteria ratings proposal (Docker) + run: node scripts/test-criteria-ratings-proposal.mjs + - name: Check generated rate schema types match the migrations run: pnpm check:db-types diff --git a/Docs/Proposals/2026-10-31-dos-me-criteria-ratings-rollback.sql b/Docs/Proposals/2026-10-31-dos-me-criteria-ratings-rollback.sql new file mode 100644 index 000000000..fd9617e59 --- /dev/null +++ b/Docs/Proposals/2026-10-31-dos-me-criteria-ratings-rollback.sql @@ -0,0 +1,42 @@ +-- PROPOSAL, NOT APPLIED. Rollback of 2026-10-31-dos-me-criteria-ratings.sql (DOS/Rate#1543). +-- One transaction, re-runnable. It gives rate.refresh_master_product_rating its previous body +-- back (the renamed base, with its owner and grants), then drops what the migration added. +-- Data written only into the new columns and tables (exact stars, verdicts, pro and con lines, +-- product points, rating_score, criteria_scores) is lost; nothing older depends on it. Run it +-- before the web reads rating_score (P4) or after that read is reverted. + +BEGIN; + +-- 1. The refresh: drop the wrapper and put the base back under its own name. +DO $$ +BEGIN + IF to_regprocedure('rate.refresh_master_product_rating_base(uuid)') IS NOT NULL THEN + DROP FUNCTION IF EXISTS rate.refresh_master_product_rating(uuid); + ALTER FUNCTION rate.refresh_master_product_rating_base(uuid) + RENAME TO refresh_master_product_rating; + END IF; +END $$; + +REVOKE EXECUTE ON FUNCTION rate.refresh_master_product_rating(uuid) FROM PUBLIC, anon, authenticated; +GRANT EXECUTE ON FUNCTION rate.refresh_master_product_rating(uuid) TO service_role, rate_worker; + +DROP FUNCTION IF EXISTS rate.refresh_master_rating_extras(uuid); + +-- 2. Tables (their policies and indexes go with them). +DROP TABLE IF EXISTS rate.product_points; +DROP TABLE IF EXISTS rate.review_points; + +-- 3. Columns (their CHECK constraints go with them). +ALTER TABLE rate.master_products + DROP COLUMN IF EXISTS criteria_scores, + DROP COLUMN IF EXISTS rating_weight_sum, + DROP COLUMN IF EXISTS rating_score; + +ALTER TABLE rate.reviews + DROP COLUMN IF EXISTS value_breakdown, + DROP COLUMN IF EXISTS value_score, + DROP COLUMN IF EXISTS criteria_version, + DROP COLUMN IF EXISTS verdict, + DROP COLUMN IF EXISTS rating_exact; + +COMMIT; diff --git a/Docs/Proposals/2026-10-31-dos-me-criteria-ratings-verify.sql b/Docs/Proposals/2026-10-31-dos-me-criteria-ratings-verify.sql new file mode 100644 index 000000000..dbcdbbd53 --- /dev/null +++ b/Docs/Proposals/2026-10-31-dos-me-criteria-ratings-verify.sql @@ -0,0 +1,64 @@ +-- PROPOSAL. Read-only checks for 2026-10-31-dos-me-criteria-ratings.sql (DOS/Rate#1543). Run +-- after it. `scripts/test-criteria-ratings-proposal.mjs` runs the expect-0 ones on the replay +-- stack. + +-- V1 Every new column exists. expect 0 +SELECT 9 - count(*) AS missing_columns +FROM information_schema.columns +WHERE table_schema = 'rate' + AND ((table_name = 'reviews' AND column_name IN ('rating_exact', 'verdict', 'criteria_version', + 'value_score', 'value_breakdown')) + OR (table_name = 'master_products' AND column_name IN ('rating_score', 'rating_weight_sum', + 'criteria_scores')) + OR (table_name = 'review_points' AND column_name = 'body')); + +-- V2 Both new tables have row level security on. expect 0 +SELECT 2 - count(*) AS tables_without_rls +FROM pg_class c JOIN pg_namespace n ON n.oid = c.relnamespace +WHERE n.nspname = 'rate' AND c.relname IN ('review_points', 'product_points') + AND c.relrowsecurity; + +-- V3 The refresh wrapper, its base and the extras exist, and anon cannot run them. expect 0 +SELECT (3 - count(*)) + count(*) FILTER (WHERE has_function_privilege('anon', p.oid, 'EXECUTE')) + AS missing_or_open_functions +FROM pg_proc p JOIN pg_namespace n ON n.oid = p.pronamespace +WHERE n.nspname = 'rate' + AND p.proname IN ('refresh_master_product_rating', 'refresh_master_product_rating_base', + 'refresh_master_rating_extras'); + +-- V4 No rating outside 0-5, and no rating without the weight behind it. expect 0 +SELECT count(*) AS bad_ratings +FROM rate.master_products +WHERE rating_score < 0 OR rating_score > 5 + OR (rating_score IS NOT NULL AND rating_weight_sum <= 0) + OR (rating_score IS NULL AND rating_weight_sum > 0); + +-- V5 Every criterion score meets the display thresholds of D9. expect 0 +SELECT count(*) AS criteria_below_threshold +FROM rate.master_products m, jsonb_each(m.criteria_scores) c(key, v) +WHERE (c.v ->> 'writers')::int < 3 + OR (c.v ->> 'weight_sum')::numeric < 1.5 + OR (c.v ->> 'score')::numeric NOT BETWEEN 1 AND 5; + +-- V6 The backfill reached every master: none with counted reviews and no rating. expect 0 +SELECT count(*) AS masters_not_backfilled +FROM rate.master_products m +WHERE m.rating_score IS NULL + AND m.authentic_review_count > 0 + AND EXISTS ( + SELECT 1 + FROM rate.products p + JOIN rate.reviews r ON r.product_id = p.id + JOIN rate.review_flags f ON f.review_id = r.id + WHERE p.cluster_id = m.id AND p.origin = 'crawl' + AND r.origin IN ('crawl', 'community') AND r.reviewer_type = 'user' + AND COALESCE(f.include_in_rating, true) AND r.helpfulness_weight > 0); + +-- V7 Information: the rating each page will read, next to today's weighted mean. +SELECT slug, user_rating_weighted, rating_score, rating_weight_sum, + jsonb_object_keys_count.n AS criteria_with_scores +FROM rate.master_products m, + LATERAL (SELECT count(*) AS n FROM jsonb_object_keys(m.criteria_scores)) jsonb_object_keys_count +WHERE m.authentic_review_count > 0 +ORDER BY m.authentic_review_count DESC +LIMIT 20; diff --git a/Docs/Proposals/2026-10-31-dos-me-criteria-ratings.md b/Docs/Proposals/2026-10-31-dos-me-criteria-ratings.md new file mode 100644 index 000000000..07b8eb0e9 --- /dev/null +++ b/Docs/Proposals/2026-10-31-dos-me-criteria-ratings.md @@ -0,0 +1,57 @@ +# Proposal: DOS-Me migration for criteria ratings, pros and cons (DOS/Rate#1543) + +**Status:** proposal for JOY, **not applied**. This is a schema change on the shared Supabase. JOY approves it and applies it through DOS-Me: preview, then beta, then prod. Nothing runs from DOS/Rate. + +Design: [`2026-10-31-criteria-stars-pros-cons.html`](./2026-10-31-criteria-stars-pros-cons.html), section 3. Phase P2 of #1531. JOY approved decisions D2, D3, D9 and D10 on 2026-10-31. + +## Files + +| File | What | +| :----------------------------------------------------------------------------------------------------- | :----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| [`2026-10-31-dos-me-criteria-ratings.sql`](./2026-10-31-dos-me-criteria-ratings.sql) | The migration. One transaction, idempotent, ends with a backfill of the new master columns. | +| [`2026-10-31-dos-me-criteria-ratings-verify.sql`](./2026-10-31-dos-me-criteria-ratings-verify.sql) | Read-only. V1 to V6 must return 0 after the migration. V7 lists `rating_score` next to today's `user_rating_weighted` for the 20 most-reviewed masters. | +| [`2026-10-31-dos-me-criteria-ratings-rollback.sql`](./2026-10-31-dos-me-criteria-ratings-rollback.sql) | Restores the previous refresh body and drops everything the migration added. One transaction, re-runnable. | +| [`scripts/test-criteria-ratings-proposal.mjs`](../../scripts/test-criteria-ratings-proposal.mjs) | CI replay in Docker. It builds a small catalog, applies the migration twice and asserts the computed rating and criteria scores. It also runs every V check, the anon and worker grants and the rollback twice, then repeats all of it on top of M0 and M1+M2, plus a full re-apply. | + +## What it adds + +- **`rate.reviews`:** + - `rating_exact numeric(2,1)`: the 0.1 star a criteria review computes. `rating_star` stays the rounded integer that existing code reads. + - `verdict` (300 characters at most). + - `criteria_version`. + - `value_score` (0 to 100) and `value_breakdown` (for P6). +- **`rate.review_points`:** one pro or con line of a review, for the whole review or for one criterion. + - A line is 5 to 150 characters, and its `method` is `writer` or `ai`. + - It holds a `cluster_id` for P5. + - Public read; only the service role writes. +- **`rate.product_points`:** the aggregated lines of a master. + - Each line has its support, writer count, an expert flag, and first and last seen dates. + - Public read; the service role and the worker (P5) write. +- **`rate.master_products`:** + - `rating_score`: the one rating of D3, Bayesian with k = 3 and mu = 4.2, the same formula as the web's `computeBayesianRatingSummary`. + - `rating_weight_sum`. + - `criteria_scores`: `{key: {score, weight_sum, writers}}`. A criterion appears only when 3 or more reviews rated it and their weight sum is 1.5 or more (D9). Readers apply the ranking threshold of 5 from `writers`. +- **`rate.refresh_master_rating_extras(master)`:** fills the three master columns. It reads the same reviews that `refresh_master_product_rating` counts. It uses `rating_exact` when a review has one, and caps each weight at 3. +- **`rate.refresh_master_product_rating(master)` becomes a wrapper:** + - The live body is renamed `refresh_master_product_rating_base`. It keeps its owner and grants, whichever version is live, including M1+M2's Item-aware one. + - The wrapper runs the base, then the extras, so every existing caller fills the new columns unchanged. + - anon and authenticated cannot run any of the three functions. + +## Order of operations + +1. **Apply order:** after M0 and M1+M2 if those are still pending. The replay covers both orders. +2. **Apply:** on preview, then run the verify file (V1 to V6 must be 0). Compare V7: `rating_score` sits a little below `user_rating_weighted` for masters with few reviews, because of the prior. That is intended (D3). +3. **Then beta, then prod**, verifying each. +4. **Keep the wrapper:** any later migration that replaces `rate.refresh_master_product_rating` must keep calling `rate.refresh_master_rating_extras`, or the new columns stop updating. +5. **What comes next:** P4 (#1545) switches the web to `rating_score`. P3 (#1544) writes the review columns and lines. P5 (#1546) writes `product_points`. + +## Rollback + +`2026-10-31-dos-me-criteria-ratings-rollback.sql` drops the wrapper and renames the base back, then drops the extras function, both tables and the new columns. Data written only into these is lost; nothing older depends on it. + +Roll back before P4 reads `rating_score`, or revert P4 first. + +## Not in this migration + +- `submit_community_review` v2 (P3) and the value score weight (P6). Each comes with its own phase. +- Per-category prior means for criteria. Version 1 uses 4.2 for every criterion. diff --git a/Docs/Proposals/2026-10-31-dos-me-criteria-ratings.sql b/Docs/Proposals/2026-10-31-dos-me-criteria-ratings.sql new file mode 100644 index 000000000..1151003bf --- /dev/null +++ b/Docs/Proposals/2026-10-31-dos-me-criteria-ratings.sql @@ -0,0 +1,251 @@ +-- PROPOSAL, NOT APPLIED. DOS-Me migration for phase P2 of the criteria ratings design +-- (DOS/Rate#1543; design Docs/Proposals/2026-10-31-criteria-stars-pros-cons.html, section 3; +-- decisions D2, D3, D9, D10 approved by JOY on 2026-10-31). JOY reviews and applies it through +-- DOS-Me (preview, then beta, then prod). Zero DDL on prod from DOS/Rate. +-- Rollback: 2026-10-31-dos-me-criteria-ratings-rollback.sql. +-- Verification: 2026-10-31-dos-me-criteria-ratings-verify.sql. +-- Replay test: scripts/test-criteria-ratings-proposal.mjs (Docker, never a real database). +-- +-- What it adds: +-- rate.reviews rating_exact (the 0.1 star a criteria review computes), verdict, +-- criteria_version, value_score, value_breakdown +-- rate.review_points one pro or con line of a review, optionally for one criterion +-- rate.product_points the aggregated pro and con lines of a master (written by P5) +-- rate.master_products rating_score (the one rating of D3), rating_weight_sum, +-- criteria_scores (per criterion, only when D9's thresholds are met) +-- rate.refresh_master_rating_extras(p_master_id) fills the three master columns. +-- rate.refresh_master_product_rating(p_master_id) becomes a wrapper: it runs the body that +-- is live today (renamed refresh_master_product_rating_base, whichever version that is, +-- M1+M2's Item-aware one included) and then the extras, so every existing caller fills the +-- new columns with no change on its side. +-- +-- Order: after M0 and M1+M2 if those are still pending. A later migration that replaces +-- rate.refresh_master_product_rating must keep calling rate.refresh_master_rating_extras. +-- +-- Idempotent: IF NOT EXISTS everywhere, constraints guarded by pg_constraint, the rename runs +-- only once (guarded by the base function's existence), CREATE OR REPLACE for functions, DROP +-- POLICY IF EXISTS before each policy. One transaction. The backfill at the end writes only +-- the three new master columns. + +BEGIN; + +-- 1. rate.reviews ---------------------------------------------------------------------------- + +ALTER TABLE rate.reviews + ADD COLUMN IF NOT EXISTS rating_exact numeric(2, 1), + ADD COLUMN IF NOT EXISTS verdict text, + ADD COLUMN IF NOT EXISTS criteria_version integer, + ADD COLUMN IF NOT EXISTS value_score numeric(5, 2), + ADD COLUMN IF NOT EXISTS value_breakdown jsonb; + +DO $$ +BEGIN + IF NOT EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'reviews_rating_exact_range') THEN + ALTER TABLE rate.reviews ADD CONSTRAINT reviews_rating_exact_range + CHECK (rating_exact IS NULL OR (rating_exact >= 1 AND rating_exact <= 5)); + END IF; + IF NOT EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'reviews_verdict_length') THEN + ALTER TABLE rate.reviews ADD CONSTRAINT reviews_verdict_length + CHECK (verdict IS NULL OR char_length(verdict) <= 300); + END IF; + IF NOT EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'reviews_criteria_version_positive') THEN + ALTER TABLE rate.reviews ADD CONSTRAINT reviews_criteria_version_positive + CHECK (criteria_version IS NULL OR criteria_version >= 1); + END IF; + IF NOT EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'reviews_value_score_range') THEN + ALTER TABLE rate.reviews ADD CONSTRAINT reviews_value_score_range + CHECK (value_score IS NULL OR (value_score >= 0 AND value_score <= 100)); + END IF; +END $$; + +-- 2. rate.review_points ------------------------------------------------------------------------ + +CREATE TABLE IF NOT EXISTS rate.review_points ( + id uuid PRIMARY KEY DEFAULT gen_random_uuid(), + review_id uuid NOT NULL REFERENCES rate.reviews(id) ON DELETE CASCADE, + -- NULL: a pro or con of the whole review; otherwise a key of + -- packages/shared-data/src/rules/rating-criteria.ts. + criterion_key text CHECK (criterion_key IS NULL OR criterion_key ~ '^[a-z_]{2,32}$'), + kind text NOT NULL CHECK (kind IN ('pro', 'con')), + body text NOT NULL CHECK (char_length(btrim(body)) BETWEEN 5 AND 150), + position smallint NOT NULL DEFAULT 0 CHECK (position BETWEEN 0 AND 19), + -- Set by the P5 aggregation; a line belongs to at most one cluster. + cluster_id uuid, + -- 'writer' for the review's own text, 'ai' for a line P5 extracts from a marketplace review. + method text NOT NULL DEFAULT 'writer' CHECK (method IN ('writer', 'ai')), + created_at timestamptz NOT NULL DEFAULT now() +); +CREATE INDEX IF NOT EXISTS review_points_review_idx ON rate.review_points (review_id); +CREATE INDEX IF NOT EXISTS review_points_cluster_idx ON rate.review_points (cluster_id) + WHERE cluster_id IS NOT NULL; + +-- 3. rate.product_points ----------------------------------------------------------------------- + +CREATE TABLE IF NOT EXISTS rate.product_points ( + id uuid PRIMARY KEY DEFAULT gen_random_uuid(), + master_id uuid NOT NULL REFERENCES rate.master_products(id) ON DELETE CASCADE, + criterion_key text CHECK (criterion_key IS NULL OR criterion_key ~ '^[a-z_]{2,32}$'), + kind text NOT NULL CHECK (kind IN ('pro', 'con')), + label text NOT NULL CHECK (char_length(btrim(label)) BETWEEN 3 AND 200), + -- Sum of the trust weights of the distinct writers behind the line, decayed by age. + support numeric(8, 2) NOT NULL DEFAULT 0 CHECK (support >= 0), + reviewer_count integer NOT NULL DEFAULT 0 CHECK (reviewer_count >= 0), + has_expert boolean NOT NULL DEFAULT false, + first_seen timestamptz NOT NULL DEFAULT now(), + last_seen timestamptz NOT NULL DEFAULT now(), + method text NOT NULL DEFAULT 'code' CHECK (method IN ('code', 'ai')), + updated_at timestamptz NOT NULL DEFAULT now() +); +CREATE INDEX IF NOT EXISTS product_points_master_idx + ON rate.product_points (master_id, kind, support DESC); + +-- 4. RLS: public read; service role writes both, the worker writes the aggregate -------------- + +ALTER TABLE rate.review_points ENABLE ROW LEVEL SECURITY; +ALTER TABLE rate.product_points ENABLE ROW LEVEL SECURITY; + +DROP POLICY IF EXISTS "Public can view review_points" ON rate.review_points; +CREATE POLICY "Public can view review_points" ON rate.review_points FOR SELECT USING (true); +DROP POLICY IF EXISTS "Service role write review_points" ON rate.review_points; +CREATE POLICY "Service role write review_points" ON rate.review_points + FOR ALL TO service_role USING (true) WITH CHECK (true); + +DROP POLICY IF EXISTS "Public can view product_points" ON rate.product_points; +CREATE POLICY "Public can view product_points" ON rate.product_points FOR SELECT USING (true); +DROP POLICY IF EXISTS "Service role write product_points" ON rate.product_points; +CREATE POLICY "Service role write product_points" ON rate.product_points + FOR ALL TO service_role USING (true) WITH CHECK (true); +DROP POLICY IF EXISTS "Worker writes product_points" ON rate.product_points; +CREATE POLICY "Worker writes product_points" ON rate.product_points + FOR ALL TO rate_worker USING (true) WITH CHECK (true); + +GRANT SELECT ON rate.review_points, rate.product_points TO anon, authenticated; +GRANT ALL ON rate.review_points, rate.product_points TO service_role; +-- The importer's P5 job writes the aggregate; review lines come only through the review RPC. +GRANT SELECT, INSERT, UPDATE, DELETE ON rate.product_points TO rate_worker; +GRANT SELECT ON rate.review_points TO rate_worker; + +-- 5. rate.master_products ---------------------------------------------------------------------- + +ALTER TABLE rate.master_products + ADD COLUMN IF NOT EXISTS rating_score numeric(3, 2), + ADD COLUMN IF NOT EXISTS rating_weight_sum numeric(10, 2) NOT NULL DEFAULT 0, + ADD COLUMN IF NOT EXISTS criteria_scores jsonb NOT NULL DEFAULT '{}'::jsonb; + +DO $$ +BEGIN + IF NOT EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'master_products_rating_score_range') THEN + ALTER TABLE rate.master_products ADD CONSTRAINT master_products_rating_score_range + CHECK (rating_score IS NULL OR (rating_score >= 0 AND rating_score <= 5)); + END IF; + IF NOT EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'master_products_rating_weight_sum_nonneg') THEN + ALTER TABLE rate.master_products ADD CONSTRAINT master_products_rating_weight_sum_nonneg + CHECK (rating_weight_sum >= 0); + END IF; +END $$; + +-- 6. The extras: one rating (D2, D3) and per-criterion scores (D9) ------------------------------ +-- +-- The same reviews as rate.refresh_master_product_rating counts: user reviews of the master's +-- crawled listings, crawl or community, included in the rating, with a weight above 0. The +-- star is rating_exact when a criteria review set it, else rating_star. Each weight is capped +-- at 3, as apps/web/src/lib/rate-domain-core.ts computeBayesianRatingSummary does. +-- +-- rating_score = (k * mu + sum(w * star)) / (k + sum(w)), k = 3, mu = 4.2; NULL with no +-- weight, so a page shows "no rating yet" instead of the prior. +-- criteria = the same estimate per criterion key of criteria_ratings, over the reviews +-- that rated it; kept only with 3 or more reviews and a weight sum of 1.5 or +-- more (D9). "writers" lets a reader apply the ranking threshold (5). +-- A Rate user writes one review per product (submit_community_review), so a +-- review is a writer. The prior mean is 4.2 until per-category means exist. + +CREATE OR REPLACE FUNCTION rate.refresh_master_rating_extras(p_master_id uuid) +RETURNS void +LANGUAGE sql +SECURITY DEFINER +SET search_path = '' +AS $$ + WITH eligible AS ( + SELECT LEAST(3.0, r.helpfulness_weight)::numeric AS w, + COALESCE(r.rating_exact, r.rating_star)::numeric AS star, + r.criteria_ratings + FROM rate.products p + JOIN rate.reviews r ON r.product_id = p.id + JOIN rate.review_flags f ON f.review_id = r.id + WHERE p.cluster_id = p_master_id + AND p.origin = 'crawl' + AND r.origin IN ('crawl', 'community') + AND r.reviewer_type = 'user' + AND COALESCE(f.include_in_rating, true) + AND r.helpfulness_weight > 0 + ), + overall AS ( + SELECT round(COALESCE(sum(w), 0), 2) AS weight_sum, + CASE WHEN sum(w) > 0 THEN round((3 * 4.2 + sum(w * star)) / (3 + sum(w)), 2) END AS score + FROM eligible + WHERE star BETWEEN 1 AND 5 + ), + per_criterion AS ( + SELECT c.key, + count(*) AS writers, + round(sum(e.w), 2) AS weight_sum, + round((3 * 4.2 + sum(e.w * (c.value #>> '{}')::numeric)) / (3 + sum(e.w)), 2) AS score + FROM eligible e + CROSS JOIN LATERAL jsonb_each( + CASE WHEN jsonb_typeof(e.criteria_ratings) = 'object' THEN e.criteria_ratings + ELSE '{}'::jsonb END + ) AS c(key, value) + WHERE jsonb_typeof(c.value) = 'number' + AND (c.value #>> '{}')::numeric BETWEEN 1 AND 5 + GROUP BY c.key + ) + UPDATE rate.master_products m + SET rating_score = o.score, + rating_weight_sum = o.weight_sum, + criteria_scores = COALESCE( + (SELECT jsonb_object_agg(pc.key, jsonb_build_object( + 'score', pc.score, 'weight_sum', pc.weight_sum, 'writers', pc.writers)) + FROM per_criterion pc + WHERE pc.writers >= 3 AND pc.weight_sum >= 1.5), + '{}'::jsonb) + FROM overall o + WHERE m.id = p_master_id; +$$; + +REVOKE EXECUTE ON FUNCTION rate.refresh_master_rating_extras(uuid) FROM PUBLIC, anon, authenticated; +GRANT EXECUTE ON FUNCTION rate.refresh_master_rating_extras(uuid) TO service_role, rate_worker; + +-- 7. Wrap the live refresh --------------------------------------------------------------------- +-- +-- Renaming keeps the live body, its owner, SECURITY DEFINER and grants, whatever version it +-- is. Only on the first run: a re-run finds the base and leaves it. + +DO $$ +BEGIN + IF to_regprocedure('rate.refresh_master_product_rating_base(uuid)') IS NULL THEN + ALTER FUNCTION rate.refresh_master_product_rating(uuid) + RENAME TO refresh_master_product_rating_base; + END IF; +END $$; + +CREATE OR REPLACE FUNCTION rate.refresh_master_product_rating(p_master_id uuid) +RETURNS void +LANGUAGE plpgsql +SECURITY DEFINER +SET search_path = '' +AS $$ +BEGIN + PERFORM rate.refresh_master_product_rating_base(p_master_id); + PERFORM rate.refresh_master_rating_extras(p_master_id); +END; +$$; + +REVOKE EXECUTE ON FUNCTION rate.refresh_master_product_rating(uuid) FROM PUBLIC, anon, authenticated; +GRANT EXECUTE ON FUNCTION rate.refresh_master_product_rating(uuid) TO service_role, rate_worker; +REVOKE EXECUTE ON FUNCTION rate.refresh_master_product_rating_base(uuid) FROM PUBLIC, anon, authenticated; + +-- 8. Backfill: every master gets its rating_score and criteria_scores once. Re-runnable. + +SELECT rate.refresh_master_rating_extras(id) FROM rate.master_products; + +COMMIT; diff --git a/scripts/test-criteria-ratings-proposal.mjs b/scripts/test-criteria-ratings-proposal.mjs new file mode 100644 index 000000000..064c1a59d --- /dev/null +++ b/scripts/test-criteria-ratings-proposal.mjs @@ -0,0 +1,209 @@ +#!/usr/bin/env node +// Replays the DOS-Me criteria ratings PROPOSAL (DOS/Rate#1543, phase P2 of #1531) on the +// migration replay stack: a small catalog with criteria reviews, the proposal twice, the +// rating and criteria scores it computes, the refresh wrapper as the worker calls it, every +// verification query, the grants, the rollback twice, then the same on top of M0 and M1+M2 +// (whose Item-aware refresh the wrapper must keep). Nothing touches a real database. +// +// Usage: node scripts/test-criteria-ratings-proposal.mjs (needs Docker) + +import assert from 'node:assert/strict'; +import { readFileSync } from 'node:fs'; +import path from 'node:path'; +import { spawnSync } from 'node:child_process'; + +import { repoRoot, startRateDb } from './lib/rate-db.mjs'; + +const read = (name) => + readFileSync(path.join(repoRoot, 'Docs', 'Proposals', name), 'utf8'); +const proposal = read('2026-10-31-dos-me-criteria-ratings.sql'); +const rollback = read('2026-10-31-dos-me-criteria-ratings-rollback.sql'); +const verify = read('2026-10-31-dos-me-criteria-ratings-verify.sql'); +const m0 = read('2026-10-29-dos-me-m0-seed-rekey.sql'); +const m1m2 = read('2026-10-28-dos-me-m1-m2-items.sql'); + +const container = `rate-criteria-proposal-${process.pid}`; +const removeContainer = () => + spawnSync('docker', ['rm', '--force', container], { timeout: 30_000 }); +for (const signal of ['SIGINT', 'SIGTERM']) { + process.on(signal, () => { + removeContainer(); + process.exit(130); + }); +} + +const expectZero = (file) => + [ + ...file.matchAll( + /^-- (V\d+) [^\n]*expect 0\n(?:--[^\n]*\n)*(SELECT[\s\S]*?;)/gm + ), + ].map(([, id, query]) => ({ id, query })); +const checks = expectZero(verify); +assert.equal(checks.length, 6, 'the verification lists its expect-0 queries'); + +const PHONES = 'c0000000-0000-4000-8000-000000000001'; +const MERCHANT = 'e0000000-0000-4000-8000-000000000001'; +const rated = 'a0000000-0000-4000-8000-000000000001'; +const empty = 'a0000000-0000-4000-8000-000000000002'; +const listing = 'b0000000-0000-4000-8000-000000000001'; +const review = (n) => `d0000000-0000-4000-8000-00000000000${n}`; + +try { + const { sql, asRole } = startRateDb({ name: container }); + const checkAll = (when) => { + for (const { id, query } of checks) + assert.equal(sql(query), '0', `${id} after ${when}`); + }; + + sql(` + INSERT INTO rate.categories (id, name, slug) VALUES ('${PHONES}', 'Điện thoại', 'smartphones'); + INSERT INTO rate.merchants (id, platform, platform_shop_id, name, slug) + VALUES ('${MERCHANT}', 'shopee', 's1', 'Shop 1', 'shop-1'); + INSERT INTO rate.master_products (id, brand, model, model_key, name, slug, category_id, status, + storage_gb, min_price, max_price) + VALUES + ('${rated}', 'Acme', 'Phone X', 'acme:phone-x:128gb', 'Acme Phone X 128GB', + 'acme-phone-x-128gb', '${PHONES}', 'published', 128, 10000000, 10000000), + ('${empty}', 'Acme', 'Phone Y', 'acme:phone-y:128gb', 'Acme Phone Y 128GB', + 'acme-phone-y-128gb', '${PHONES}', 'published', 128, 9000000, 9000000); + INSERT INTO rate.products (id, merchant_id, platform, platform_item_id, title, price_current, url, cluster_id) + VALUES ('${listing}', '${MERCHANT}', 'shopee', 'x1', 'Acme Phone X 128GB', 10000000, + 'https://shopee.vn/x1', '${rated}'); + -- Three criteria reviews (one with its exact 0.1 star), one excluded from the rating, + -- one marketplace review with no criteria and weight 2. + INSERT INTO rate.reviews (id, product_id, rating_star, comment, reviewer_type, criteria_ratings) + VALUES + ('${review(1)}', '${listing}', 5, 'Màn hình đẹp, pin ổn.', 'user', '{"display": 5, "value": 4}'), + ('${review(2)}', '${listing}', 4, 'Dùng tốt trong tầm giá.', 'user', '{"display": 4, "value": 4}'), + ('${review(3)}', '${listing}', 3, 'Màn hơi tối ngoài nắng.', 'user', '{"display": 3}'), + ('${review(4)}', '${listing}', 1, 'Giao sai màu.', 'user', '{"display": 1, "value": 1}'), + ('${review(5)}', '${listing}', 5, 'Máy chạy mượt.', 'user', '{}'); + INSERT INTO rate.review_flags (review_id, verdict, include_in_rating, detector, detector_version, input_hash) + VALUES + ('${review(1)}', 'authentic', true, 'replay', 'v1', 'h1'), + ('${review(2)}', 'authentic', true, 'replay', 'v1', 'h2'), + ('${review(3)}', 'authentic', true, 'replay', 'v1', 'h3'), + ('${review(4)}', 'suspicious', false, 'replay', 'v1', 'h4'), + ('${review(5)}', 'authentic', true, 'replay', 'v1', 'h5'); + UPDATE rate.reviews SET helpfulness_weight = 1 + WHERE id IN ('${review(1)}', '${review(2)}', '${review(3)}', '${review(4)}'); + UPDATE rate.reviews SET helpfulness_weight = 2 WHERE id = '${review(5)}'; + SELECT rate.refresh_master_product_rating('${rated}'); + `); + + const masterRow = (id) => + sql( + `SELECT coalesce(rating_score::text, '-') || '|' || rating_weight_sum || '|' || criteria_scores::text + FROM rate.master_products WHERE id = '${id}'` + ); + // Weights 1 + 1 + 1 + 2 = 5 (review 4 is excluded); stars 5, 4.3 (exact), 3, 5. + // (3 * 4.2 + 5 + 4.3 + 3 + 10) / (3 + 5) = 34.9 / 8 = 4.36. + // display: 3 writers, weight 3: (12.6 + 5 + 4 + 3) / 6 = 4.10. value: 2 writers, hidden. + const expected = + '4.36|5.00|{"display": {"score": 4.10, "writers": 3, "weight_sum": 3.00}}'; + + const applyAndCheck = (when) => { + sql(proposal); + sql(proposal); + sql(`UPDATE rate.reviews SET rating_exact = 4.3 WHERE id = '${review(2)}'`); + // The worker's usual call goes through the wrapper: the live body, then the extras. + asRole( + 'rate_worker', + `SELECT rate.refresh_master_product_rating('${rated}')` + ); + assert.equal(masterRow(rated), expected, `rating and criteria ${when}`); + assert.equal( + masterRow(empty), + '-|0.00|{}', + `a master with no reviews ${when}` + ); + assert.equal( + sql( + `SELECT user_rating_weighted FROM rate.master_products WHERE id = '${rated}'` + ), + '4.40', + `the live body still fills user_rating_weighted ${when}` + ); + checkAll(when); + for (const fn of [ + `rate.refresh_master_rating_extras('${rated}')`, + `rate.refresh_master_product_rating('${rated}')`, + `rate.refresh_master_product_rating_base('${rated}')`, + ]) + assert.throws( + () => asRole('anon', `SELECT ${fn}`), + /permission denied/, + `anon ${fn}` + ); + assert.equal( + asRole('anon', `SELECT count(*) FROM rate.review_points`), + '0', + `anon reads review_points ${when}` + ); + assert.throws( + () => + asRole( + 'anon', + `INSERT INTO rate.product_points (master_id, kind, label) VALUES ('${rated}', 'pro', 'Pin tốt')` + ), + /permission denied|row-level security/, + `anon cannot write product_points ${when}` + ); + asRole( + 'rate_worker', + `INSERT INTO rate.product_points (master_id, kind, label, support, reviewer_count) + VALUES ('${rated}', 'pro', 'Màn hình đẹp', 2, 2)` + ); + assert.throws( + () => + sql( + `INSERT INTO rate.review_points (review_id, kind, body) VALUES ('${review(1)}', 'pro', 'ok')` + ), + /review_points_body_check/, + `a pro line under 5 characters is refused ${when}` + ); + }; + + const rollBackAndCheck = (when) => { + sql(rollback); + sql(rollback); + assert.equal( + sql(`SELECT count(*) FROM information_schema.columns + WHERE table_schema = 'rate' AND column_name IN ('rating_exact', 'rating_score', 'criteria_scores')`), + '0', + `columns dropped ${when}` + ); + assert.equal( + sql(`SELECT to_regclass('rate.review_points') IS NULL AND to_regclass('rate.product_points') IS NULL + AND to_regprocedure('rate.refresh_master_product_rating_base(uuid)') IS NULL`), + 't', + `tables and the base name gone ${when}` + ); + asRole( + 'rate_worker', + `SELECT rate.refresh_master_product_rating('${rated}')` + ); + assert.equal( + sql( + `SELECT user_rating_weighted FROM rate.master_products WHERE id = '${rated}'` + ), + '4.40', + `the restored refresh still works ${when}` + ); + }; + + applyAndCheck('on the reference stack'); + rollBackAndCheck('on the reference stack'); + + // The order JOY applies them in: M0, M1+M2 (an Item-aware refresh), then this proposal. + sql(m0); + sql(m1m2); + applyAndCheck('after M0 and M1+M2'); + rollBackAndCheck('after M0 and M1+M2'); + // A full re-apply after a rollback. + applyAndCheck('re-applied after a rollback'); + + console.log('criteria ratings proposal: apply, verify and rollback replayed'); +} finally { + removeContainer(); +} From f75dbcbc448d46a03495ebe25c39bc1f42861509 Mon Sep 17 00:00:00 2001 From: JOY <5027251+JOY@users.noreply.github.com> Date: Sun, 1 Nov 2026 01:57:09 +0700 Subject: [PATCH 2/5] fix(proposals): keep criteria ratings current with triggers, not a wrapper Review of #1549. Renaming refresh_master_product_rating to a base and wrapping it broke when M1+M2 or its rollback replaced the function, and the Item path (refresh_item_rating, votes) bypassed it. The migration now replaces no function: triggers on reviews, review_flags and master_products refresh the new columns, scoped like the live refresh (per master, or the whole Item once M1+M2 is in). The schema change and the backfill commit separately; criteria keys must look like the shared-data keys; V3 checks PUBLIC and authenticated too, V4 the triggers, and V6 recomputes every master. The replay applies it before and after M1+M2 and through the M1+M2 rollback. Refs #1543 Co-Authored-By: Claude Opus 5.5 --- ...10-31-dos-me-criteria-ratings-rollback.sql | 44 ++- ...6-10-31-dos-me-criteria-ratings-verify.sql | 59 +-- .../2026-10-31-dos-me-criteria-ratings.md | 91 +++-- .../2026-10-31-dos-me-criteria-ratings.sql | 345 +++++++++++++----- scripts/test-criteria-ratings-proposal.mjs | 251 +++++++++---- 5 files changed, 549 insertions(+), 241 deletions(-) diff --git a/Docs/Proposals/2026-10-31-dos-me-criteria-ratings-rollback.sql b/Docs/Proposals/2026-10-31-dos-me-criteria-ratings-rollback.sql index fd9617e59..74b1e06d8 100644 --- a/Docs/Proposals/2026-10-31-dos-me-criteria-ratings-rollback.sql +++ b/Docs/Proposals/2026-10-31-dos-me-criteria-ratings-rollback.sql @@ -1,32 +1,36 @@ -- PROPOSAL, NOT APPLIED. Rollback of 2026-10-31-dos-me-criteria-ratings.sql (DOS/Rate#1543). --- One transaction, re-runnable. It gives rate.refresh_master_product_rating its previous body --- back (the renamed base, with its owner and grants), then drops what the migration added. --- Data written only into the new columns and tables (exact stars, verdicts, pro and con lines, --- product points, rating_score, criteria_scores) is lost; nothing older depends on it. Run it --- before the web reads rating_score (P4) or after that read is reverted. +-- One transaction, re-runnable, safe before or after M1+M2 or its rollback: the migration +-- replaces no existing function, so this only drops what it added. Data written only into the +-- new columns and tables (exact stars, verdicts, pro and con lines, product points, +-- rating_score, criteria_scores) is lost; nothing older depends on it. Run it only while no +-- phase reads or writes them: before P3 (#1544), P4 (#1545) and P5 (#1546), or after they are +-- reverted. BEGIN; --- 1. The refresh: drop the wrapper and put the base back under its own name. -DO $$ -BEGIN - IF to_regprocedure('rate.refresh_master_product_rating_base(uuid)') IS NOT NULL THEN - DROP FUNCTION IF EXISTS rate.refresh_master_product_rating(uuid); - ALTER FUNCTION rate.refresh_master_product_rating_base(uuid) - RENAME TO refresh_master_product_rating; - END IF; -END $$; - -REVOKE EXECUTE ON FUNCTION rate.refresh_master_product_rating(uuid) FROM PUBLIC, anon, authenticated; -GRANT EXECUTE ON FUNCTION rate.refresh_master_product_rating(uuid) TO service_role, rate_worker; - +-- 1. Triggers first, so the drops below fire nothing. +DROP TRIGGER IF EXISTS trg_reviews_rating_extras_ins ON rate.reviews; +DROP TRIGGER IF EXISTS trg_reviews_rating_extras_upd ON rate.reviews; +DROP TRIGGER IF EXISTS trg_reviews_rating_extras_del ON rate.reviews; +DROP TRIGGER IF EXISTS trg_review_flags_rating_extras_ins ON rate.review_flags; +DROP TRIGGER IF EXISTS trg_review_flags_rating_extras_upd ON rate.review_flags; +DROP TRIGGER IF EXISTS trg_review_flags_rating_extras_del ON rate.review_flags; +DROP TRIGGER IF EXISTS trg_master_products_rating_extras ON rate.master_products; + +-- 2. Functions. +DROP FUNCTION IF EXISTS rate.master_products_refresh_rating_extras(); +DROP FUNCTION IF EXISTS rate.review_flags_refresh_rating_extras(); +DROP FUNCTION IF EXISTS rate.reviews_refresh_rating_extras(); +DROP FUNCTION IF EXISTS rate.refresh_rating_extras_of_masters(uuid[]); DROP FUNCTION IF EXISTS rate.refresh_master_rating_extras(uuid); +DROP FUNCTION IF EXISTS rate.master_rating_extras(uuid); +DROP FUNCTION IF EXISTS rate.master_rating_item(uuid); --- 2. Tables (their policies and indexes go with them). +-- 3. Tables (their policies and indexes go with them). DROP TABLE IF EXISTS rate.product_points; DROP TABLE IF EXISTS rate.review_points; --- 3. Columns (their CHECK constraints go with them). +-- 4. Columns (their CHECK constraints go with them). ALTER TABLE rate.master_products DROP COLUMN IF EXISTS criteria_scores, DROP COLUMN IF EXISTS rating_weight_sum, diff --git a/Docs/Proposals/2026-10-31-dos-me-criteria-ratings-verify.sql b/Docs/Proposals/2026-10-31-dos-me-criteria-ratings-verify.sql index dbcdbbd53..84dcaf9b5 100644 --- a/Docs/Proposals/2026-10-31-dos-me-criteria-ratings-verify.sql +++ b/Docs/Proposals/2026-10-31-dos-me-criteria-ratings-verify.sql @@ -18,20 +18,30 @@ FROM pg_class c JOIN pg_namespace n ON n.oid = c.relnamespace WHERE n.nspname = 'rate' AND c.relname IN ('review_points', 'product_points') AND c.relrowsecurity; --- V3 The refresh wrapper, its base and the extras exist, and anon cannot run them. expect 0 -SELECT (3 - count(*)) + count(*) FILTER (WHERE has_function_privilege('anon', p.oid, 'EXECUTE')) +-- V3 Each new function exists once, and no client role can run it. expect 0 +-- Client roles: PUBLIC, anon, authenticated. +SELECT count(*) FILTER (WHERE to_regprocedure(w.signature) IS NULL) + + count(*) FILTER (WHERE to_regprocedure(w.signature) IS NOT NULL AND ( + has_function_privilege('anon', to_regprocedure(w.signature), 'EXECUTE') + OR has_function_privilege('authenticated', to_regprocedure(w.signature), 'EXECUTE') + OR EXISTS (SELECT 1 FROM pg_proc p, aclexplode(p.proacl) a + WHERE p.oid = to_regprocedure(w.signature) AND a.grantee = 0))) AS missing_or_open_functions -FROM pg_proc p JOIN pg_namespace n ON n.oid = p.pronamespace -WHERE n.nspname = 'rate' - AND p.proname IN ('refresh_master_product_rating', 'refresh_master_product_rating_base', - 'refresh_master_rating_extras'); +FROM (VALUES ('rate.master_rating_item(uuid)'), ('rate.master_rating_extras(uuid)'), + ('rate.refresh_master_rating_extras(uuid)'), + ('rate.refresh_rating_extras_of_masters(uuid[])'), + ('rate.reviews_refresh_rating_extras()'), + ('rate.review_flags_refresh_rating_extras()'), + ('rate.master_products_refresh_rating_extras()')) w(signature); --- V4 No rating outside 0-5, and no rating without the weight behind it. expect 0 -SELECT count(*) AS bad_ratings -FROM rate.master_products -WHERE rating_score < 0 OR rating_score > 5 - OR (rating_score IS NOT NULL AND rating_weight_sum <= 0) - OR (rating_score IS NULL AND rating_weight_sum > 0); +-- V4 All seven triggers exist and are enabled. expect 0 +SELECT 7 - count(*) AS missing_triggers +FROM pg_trigger t JOIN pg_class c ON c.oid = t.tgrelid JOIN pg_namespace n ON n.oid = c.relnamespace +WHERE n.nspname = 'rate' AND NOT t.tgisinternal AND t.tgenabled <> 'D' + AND t.tgname IN ('trg_reviews_rating_extras_ins', 'trg_reviews_rating_extras_upd', + 'trg_reviews_rating_extras_del', 'trg_review_flags_rating_extras_ins', + 'trg_review_flags_rating_extras_upd', 'trg_review_flags_rating_extras_del', + 'trg_master_products_rating_extras'); -- V5 Every criterion score meets the display thresholds of D9. expect 0 SELECT count(*) AS criteria_below_threshold @@ -40,25 +50,18 @@ WHERE (c.v ->> 'writers')::int < 3 OR (c.v ->> 'weight_sum')::numeric < 1.5 OR (c.v ->> 'score')::numeric NOT BETWEEN 1 AND 5; --- V6 The backfill reached every master: none with counted reviews and no rating. expect 0 -SELECT count(*) AS masters_not_backfilled +-- V6 Every master holds what its reviews give today. expect 0 +-- No stale, missing or wrong rating or criteria: catches a skipped backfill and a missed trigger. +SELECT count(*) AS masters_out_of_date FROM rate.master_products m -WHERE m.rating_score IS NULL - AND m.authentic_review_count > 0 - AND EXISTS ( - SELECT 1 - FROM rate.products p - JOIN rate.reviews r ON r.product_id = p.id - JOIN rate.review_flags f ON f.review_id = r.id - WHERE p.cluster_id = m.id AND p.origin = 'crawl' - AND r.origin IN ('crawl', 'community') AND r.reviewer_type = 'user' - AND COALESCE(f.include_in_rating, true) AND r.helpfulness_weight > 0); +CROSS JOIN LATERAL rate.master_rating_extras(m.id) x +WHERE (m.rating_score, m.rating_weight_sum, m.criteria_scores) + IS DISTINCT FROM (x.rating_score, x.rating_weight_sum, x.criteria_scores); -- V7 Information: the rating each page will read, next to today's weighted mean. -SELECT slug, user_rating_weighted, rating_score, rating_weight_sum, - jsonb_object_keys_count.n AS criteria_with_scores -FROM rate.master_products m, - LATERAL (SELECT count(*) AS n FROM jsonb_object_keys(m.criteria_scores)) jsonb_object_keys_count +SELECT m.slug, m.user_rating_weighted, m.rating_score, m.rating_weight_sum, + (SELECT count(*) FROM jsonb_object_keys(m.criteria_scores)) AS criteria_with_scores +FROM rate.master_products m WHERE m.authentic_review_count > 0 ORDER BY m.authentic_review_count DESC LIMIT 20; diff --git a/Docs/Proposals/2026-10-31-dos-me-criteria-ratings.md b/Docs/Proposals/2026-10-31-dos-me-criteria-ratings.md index 07b8eb0e9..fa262a0da 100644 --- a/Docs/Proposals/2026-10-31-dos-me-criteria-ratings.md +++ b/Docs/Proposals/2026-10-31-dos-me-criteria-ratings.md @@ -1,57 +1,82 @@ # Proposal: DOS-Me migration for criteria ratings, pros and cons (DOS/Rate#1543) -**Status:** proposal for JOY, **not applied**. This is a schema change on the shared Supabase. JOY approves it and applies it through DOS-Me: preview, then beta, then prod. Nothing runs from DOS/Rate. +**Status:** proposal for JOY, **not applied**. A schema change on the shared Supabase: JOY approves and applies it through DOS-Me (preview, then beta, then prod). Nothing runs from DOS/Rate. -Design: [`2026-10-31-criteria-stars-pros-cons.html`](./2026-10-31-criteria-stars-pros-cons.html), section 3. Phase P2 of #1531. JOY approved decisions D2, D3, D9 and D10 on 2026-10-31. +Design: [`2026-10-31-criteria-stars-pros-cons.html`](./2026-10-31-criteria-stars-pros-cons.html), section 3; phase P2 of #1531, decisions D2, D3, D9 and D10 approved by JOY on 2026-10-31. ## Files -| File | What | -| :----------------------------------------------------------------------------------------------------- | :----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| [`2026-10-31-dos-me-criteria-ratings.sql`](./2026-10-31-dos-me-criteria-ratings.sql) | The migration. One transaction, idempotent, ends with a backfill of the new master columns. | -| [`2026-10-31-dos-me-criteria-ratings-verify.sql`](./2026-10-31-dos-me-criteria-ratings-verify.sql) | Read-only. V1 to V6 must return 0 after the migration. V7 lists `rating_score` next to today's `user_rating_weighted` for the 20 most-reviewed masters. | -| [`2026-10-31-dos-me-criteria-ratings-rollback.sql`](./2026-10-31-dos-me-criteria-ratings-rollback.sql) | Restores the previous refresh body and drops everything the migration added. One transaction, re-runnable. | -| [`scripts/test-criteria-ratings-proposal.mjs`](../../scripts/test-criteria-ratings-proposal.mjs) | CI replay in Docker. It builds a small catalog, applies the migration twice and asserts the computed rating and criteria scores. It also runs every V check, the anon and worker grants and the rollback twice, then repeats all of it on top of M0 and M1+M2, plus a full re-apply. | +- [`2026-10-31-dos-me-criteria-ratings.sql`](./2026-10-31-dos-me-criteria-ratings.sql): the migration. Idempotent, in two transactions: the schema change first, then a backfill of the reviewed masters. +- [`2026-10-31-dos-me-criteria-ratings-verify.sql`](./2026-10-31-dos-me-criteria-ratings-verify.sql): read-only checks. V1 to V6 must return 0 after applying: + - V3: function grants for PUBLIC, anon and authenticated. + - V4: the triggers. + - V6: recomputes every master and compares it with what is stored. + - V7: lists `rating_score` next to today's `user_rating_weighted` for the 20 most-reviewed masters. +- [`2026-10-31-dos-me-criteria-ratings-rollback.sql`](./2026-10-31-dos-me-criteria-ratings-rollback.sql): drops everything the migration added. One transaction, re-runnable. +- [`scripts/test-criteria-ratings-proposal.mjs`](../../scripts/test-criteria-ratings-proposal.mjs): the CI replay in Docker, in four steps: + 1. **Reference stack:** apply twice; check the ratings, the triggers (exact star, a flag that leaves the rating, the weight cap) and the grants; roll back twice. + 2. **Before M1+M2:** apply, then run M0 and M1+M2. Two sizes become one Item, both variants show the pooled rating, and a vote on the Item path updates it. + 3. **M1+M2 rollback** with this migration in place. + 4. **After M1+M2:** M1+M2 again, then this migration on top, then the rollback. ## What it adds - **`rate.reviews`:** - - `rating_exact numeric(2,1)`: the 0.1 star a criteria review computes. `rating_star` stays the rounded integer that existing code reads. + - `rating_exact numeric(2,1)`: the 0.1-step star a criteria review computes. `rating_star` stays the rounded integer that existing code reads. - `verdict` (300 characters at most). - `criteria_version`. - - `value_score` (0 to 100) and `value_breakdown` (for P6). -- **`rate.review_points`:** one pro or con line of a review, for the whole review or for one criterion. - - A line is 5 to 150 characters, and its `method` is `writer` or `ai`. - - It holds a `cluster_id` for P5. + - `value_score` (0 to 100) and `value_breakdown`, for P6. +- **`rate.review_points`:** one pro or con line of a review, for the whole review or one criterion. + - Each line is 5 to 150 characters. + - `method` is `writer` or `ai`. + - `cluster_id` is filled by P5. - Public read; only the service role writes. -- **`rate.product_points`:** the aggregated lines of a master. - - Each line has its support, writer count, an expert flag, and first and last seen dates. - - Public read; the service role and the worker (P5) write. +- **`rate.product_points`:** the aggregated lines of a master, with support, writer count, an expert flag, and first and last seen dates. Public read; the service role and the worker (P5) write. - **`rate.master_products`:** - - `rating_score`: the one rating of D3, Bayesian with k = 3 and mu = 4.2, the same formula as the web's `computeBayesianRatingSummary`. + - `rating_score`: the one rating of D3. It is a Bayesian average with k = 3 and mu = 4.2 (the same formula as the web's `computeBayesianRatingSummary`). Each review weight is capped at 3, and `rating_exact` is used when present. - `rating_weight_sum`. - - `criteria_scores`: `{key: {score, weight_sum, writers}}`. A criterion appears only when 3 or more reviews rated it and their weight sum is 1.5 or more (D9). Readers apply the ranking threshold of 5 from `writers`. -- **`rate.refresh_master_rating_extras(master)`:** fills the three master columns. It reads the same reviews that `refresh_master_product_rating` counts. It uses `rating_exact` when a review has one, and caps each weight at 3. -- **`rate.refresh_master_product_rating(master)` becomes a wrapper:** - - The live body is renamed `refresh_master_product_rating_base`. It keeps its owner and grants, whichever version is live, including M1+M2's Item-aware one. - - The wrapper runs the base, then the extras, so every existing caller fills the new columns unchanged. - - anon and authenticated cannot run any of the three functions. + - `criteria_scores`: `{key: {score, weight_sum, writers}}`. A criterion appears only when all of these hold (D9): + - 3 or more reviews rated it; + - each rating is a number from 1 to 5, under a key shaped like the shared-data keys; + - the reviews' weight sum is 1.5 or more. + + Readers apply the ranking threshold of 5 from `writers`. +- **Which reviews count:** the same reviews `refresh_master_product_rating` counts. + - Before M1+M2: the master's own listings. + - Once the master belongs to an Item: every listing of the Item, and the result is written to every variant, as `refresh_item_rating` does. + - The `item_id` columns are read dynamically, so the same functions work with or without M1+M2. +- **Functions:** + - `rate.master_rating_extras(master)` computes the values. It is read-only; the verify file uses it. + - `rate.refresh_master_rating_extras(master)` writes them, only where they changed. + - Only the service role can run them. +- **Triggers keep the values current.** + - On `rate.reviews` and `rate.review_flags`: every insert, update and delete. They run once per statement, so a bulk import refreshes each touched master once. + - On `rate.master_products`: a change of `user_rating_weighted` or `authentic_review_count`, for example after a listing moves. + - **No existing function is replaced or renamed.** A later `CREATE OR REPLACE` of a refresh function (M1+M2, its rollback, a hotfix) cannot stop these updates. ## Order of operations -1. **Apply order:** after M0 and M1+M2 if those are still pending. The replay covers both orders. -2. **Apply:** on preview, then run the verify file (V1 to V6 must be 0). Compare V7: `rating_score` sits a little below `user_rating_weighted` for masters with few reviews, because of the prior. That is intended (D3). -3. **Then beta, then prod**, verifying each. -4. **Keep the wrapper:** any later migration that replaces `rate.refresh_master_product_rating` must keep calling `rate.refresh_master_rating_extras`, or the new columns stop updating. -5. **What comes next:** P4 (#1545) switches the web to `rating_score`. P3 (#1544) writes the review columns and lines. P5 (#1546) writes `product_points`. +1. **Apply order:** before or after M0 and M1+M2. The replay covers both orders and the M1+M2 rollback. +2. **Apply on preview:** + - The schema transaction briefly takes ACCESS EXCLUSIVE locks on `rate.reviews` and `rate.master_products` (the `ALTER TABLE` statements). + - The backfill runs afterwards in its own transaction and locks only reviewed master rows. + - Run the verify file: V1 to V6 must return 0. + - Compare V7: `rating_score` sits a little below `user_rating_weighted` for masters with few reviews, because of the prior. That is intended (D3). +3. **Then beta, then prod**, running the verify file after each. +4. **If M1+M2 is ever rolled back while this is in place:** run the backfill again (section 8 of the migration, its last transaction), then V6. Variants whose reviews did not change keep the Item's pooled values until then; V6 lists them. +5. **Next phases:** + - P4 (#1545) switches the web to `rating_score`. + - P3 (#1544) writes the new review columns and lines. + - P5 (#1546) writes `product_points`. ## Rollback -`2026-10-31-dos-me-criteria-ratings-rollback.sql` drops the wrapper and renames the base back, then drops the extras function, both tables and the new columns. Data written only into these is lost; nothing older depends on it. +`2026-10-31-dos-me-criteria-ratings-rollback.sql` drops the triggers, the functions, both tables and the new columns. It is safe before or after M1+M2, because the migration replaced nothing. Data written only into these objects is lost; nothing older depends on it. -Roll back before P4 reads `rating_score`, or revert P4 first. +Roll back only while no phase reads or writes the new columns: before P3, P4 and P5, or after reverting them. -## Not in this migration +## Known limits -- `submit_community_review` v2 (P3) and the value score weight (P6). Each comes with its own phase. -- Per-category prior means for criteria. Version 1 uses 4.2 for every criterion. +- **`rate_worker` can write the new columns directly.** It keeps its table-level UPDATE on `master_products` and on crawled reviews (20261018183000). The next trigger run overwrites such a write, and V6 shows it. Column-level grants are a separate hardening step. +- **Per-category prior means:** version 1 uses 4.2 for every criterion. +- **Not in this migration:** `submit_community_review` v2 (P3) and the value score weight (P6). Each comes with its own phase. diff --git a/Docs/Proposals/2026-10-31-dos-me-criteria-ratings.sql b/Docs/Proposals/2026-10-31-dos-me-criteria-ratings.sql index 1151003bf..5ed5a2962 100644 --- a/Docs/Proposals/2026-10-31-dos-me-criteria-ratings.sql +++ b/Docs/Proposals/2026-10-31-dos-me-criteria-ratings.sql @@ -13,19 +13,19 @@ -- rate.product_points the aggregated pro and con lines of a master (written by P5) -- rate.master_products rating_score (the one rating of D3), rating_weight_sum, -- criteria_scores (per criterion, only when D9's thresholds are met) --- rate.refresh_master_rating_extras(p_master_id) fills the three master columns. --- rate.refresh_master_product_rating(p_master_id) becomes a wrapper: it runs the body that --- is live today (renamed refresh_master_product_rating_base, whichever version that is, --- M1+M2's Item-aware one included) and then the extras, so every existing caller fills the --- new columns with no change on its side. +-- rate.master_rating_extras(p_master_id) computes the three master columns (read-only); +-- rate.refresh_master_rating_extras(p_master_id) writes them, to the master or, once M1+M2 +-- is applied, to every variant of its Item (the pooled rating, like refresh_item_rating). +-- Triggers on rate.reviews, rate.review_flags and rate.master_products keep them current. +-- No existing function is replaced or renamed. -- --- Order: after M0 and M1+M2 if those are still pending. A later migration that replaces --- rate.refresh_master_product_rating must keep calling rate.refresh_master_rating_extras. +-- Order: before or after M0 and M1+M2; the replay test applies it both ways, and through the +-- M1+M2 rollback. -- --- Idempotent: IF NOT EXISTS everywhere, constraints guarded by pg_constraint, the rename runs --- only once (guarded by the base function's existence), CREATE OR REPLACE for functions, DROP --- POLICY IF EXISTS before each policy. One transaction. The backfill at the end writes only --- the three new master columns. +-- Idempotent: IF NOT EXISTS everywhere, constraints guarded by pg_constraint, CREATE OR +-- REPLACE for functions, DROP ... IF EXISTS before each policy and trigger. Two transactions: +-- the schema change, then the backfill (section 8), which writes only the three new master +-- columns of reviewed masters. BEGIN; @@ -144,108 +144,275 @@ BEGIN END IF; END $$; --- 6. The extras: one rating (D2, D3) and per-criterion scores (D9) ------------------------------ +-- 6. The rating and criteria scores of a master (D2, D3, D9) ----------------------------------- -- --- The same reviews as rate.refresh_master_product_rating counts: user reviews of the master's --- crawled listings, crawl or community, included in the rating, with a weight above 0. The --- star is rating_exact when a criteria review set it, else rating_star. Each weight is capped --- at 3, as apps/web/src/lib/rate-domain-core.ts computeBayesianRatingSummary does. +-- The reviews rate.refresh_master_product_rating counts: user reviews of crawled listings, +-- crawl or community, included in the rating, with a weight above 0. Their scope follows the +-- live refresh: the master's own listings (cluster_id) before M1+M2, the whole Item's listings +-- (item_id) once the master belongs to an Item, so every variant shows the Item's pooled +-- rating, as refresh_item_rating does for user_rating_weighted. The item_id columns are read +-- dynamically, so the same functions work with and without M1+M2. -- --- rating_score = (k * mu + sum(w * star)) / (k + sum(w)), k = 3, mu = 4.2; NULL with no --- weight, so a page shows "no rating yet" instead of the prior. --- criteria = the same estimate per criterion key of criteria_ratings, over the reviews --- that rated it; kept only with 3 or more reviews and a weight sum of 1.5 or --- more (D9). "writers" lets a reader apply the ranking threshold (5). --- A Rate user writes one review per product (submit_community_review), so a --- review is a writer. The prior mean is 4.2 until per-category means exist. +-- rating_score = (k * mu + sum(w * star)) / (k + sum(w)), k = 3, mu = 4.2, each weight capped +-- at 3 and the star = rating_exact when a criteria review set it, else +-- rating_star (apps/web/src/lib/rate-domain-core.ts computeBayesianRatingSummary). +-- NULL with no weight, so a page shows "no rating yet" instead of the prior. +-- criteria = the same estimate per criterion of criteria_ratings, over the reviews that +-- rated it with a number from 1 to 5 under a key shaped like the shared-data +-- keys (^[a-z_]{2,32}$); kept only with 3 or more reviews and a weight sum of +-- 1.5 or more (D9). "writers" lets a reader apply the ranking threshold (5). A +-- Rate user writes one review per product, so a review is a writer. The prior +-- mean is 4.2 for every criterion until per-category means exist. +-- The Item a master belongs to, or NULL (always NULL before M1+M2). +CREATE OR REPLACE FUNCTION rate.master_rating_item(p_master_id uuid) +RETURNS uuid +LANGUAGE plpgsql +STABLE +SECURITY DEFINER +SET search_path = '' +AS $$ +DECLARE + v_item uuid; +BEGIN + IF EXISTS (SELECT 1 FROM pg_catalog.pg_attribute + WHERE attrelid = 'rate.master_products'::pg_catalog.regclass + AND attname = 'item_id' AND NOT attisdropped) THEN + EXECUTE 'SELECT item_id FROM rate.master_products WHERE id = $1' INTO v_item USING p_master_id; + END IF; + RETURN v_item; +END $$; + +-- What the master's rating columns should hold. Read-only: the verify file compares it with +-- the stored columns. +CREATE OR REPLACE FUNCTION rate.master_rating_extras(p_master_id uuid) +RETURNS TABLE (rating_score numeric, rating_weight_sum numeric, criteria_scores jsonb) +LANGUAGE plpgsql +STABLE +SECURITY DEFINER +SET search_path = '' +AS $$ +DECLARE + v_item uuid := rate.master_rating_item(p_master_id); +BEGIN + RETURN QUERY EXECUTE format($q$ + WITH eligible AS ( + SELECT LEAST(3.0, r.helpfulness_weight)::numeric AS w, + COALESCE(r.rating_exact, r.rating_star)::numeric AS star, + r.criteria_ratings + FROM rate.products p + JOIN rate.reviews r ON r.product_id = p.id + JOIN rate.review_flags f ON f.review_id = r.id + WHERE %s = $1 + AND p.origin = 'crawl' + AND r.origin IN ('crawl', 'community') + AND r.reviewer_type = 'user' + AND COALESCE(f.include_in_rating, true) + AND r.helpfulness_weight > 0 + ), + overall AS ( + SELECT round(COALESCE(sum(w), 0), 2) AS weight_sum, + CASE WHEN sum(w) > 0 THEN round((3 * 4.2 + sum(w * star)) / (3 + sum(w)), 2) END AS score + FROM eligible + WHERE star BETWEEN 1 AND 5 + ), + per_criterion AS ( + SELECT c.key, + count(*) AS writers, + round(sum(e.w), 2) AS weight_sum, + round((3 * 4.2 + sum(e.w * (c.value #>> '{}')::numeric)) / (3 + sum(e.w)), 2) AS score + FROM eligible e + CROSS JOIN LATERAL jsonb_each( + CASE WHEN jsonb_typeof(e.criteria_ratings) = 'object' THEN e.criteria_ratings + ELSE '{}'::jsonb END + ) AS c(key, value) + WHERE c.key ~ '^[a-z_]{2,32}$' + AND jsonb_typeof(c.value) = 'number' + AND (c.value #>> '{}')::numeric BETWEEN 1 AND 5 + GROUP BY c.key + ) + SELECT o.score, + o.weight_sum, + COALESCE( + (SELECT jsonb_object_agg(pc.key, jsonb_build_object( + 'score', pc.score, 'weight_sum', pc.weight_sum, 'writers', pc.writers)) + FROM per_criterion pc + WHERE pc.writers >= 3 AND pc.weight_sum >= 1.5), + '{}'::jsonb) + FROM overall o + $q$, CASE WHEN v_item IS NULL THEN 'p.cluster_id' ELSE 'p.item_id' END) + USING COALESCE(v_item, p_master_id); +END $$; + +-- Writes them, to the master or to every variant of its Item, only where they changed. CREATE OR REPLACE FUNCTION rate.refresh_master_rating_extras(p_master_id uuid) RETURNS void -LANGUAGE sql +LANGUAGE plpgsql SECURITY DEFINER SET search_path = '' AS $$ - WITH eligible AS ( - SELECT LEAST(3.0, r.helpfulness_weight)::numeric AS w, - COALESCE(r.rating_exact, r.rating_star)::numeric AS star, - r.criteria_ratings - FROM rate.products p - JOIN rate.reviews r ON r.product_id = p.id - JOIN rate.review_flags f ON f.review_id = r.id - WHERE p.cluster_id = p_master_id - AND p.origin = 'crawl' - AND r.origin IN ('crawl', 'community') - AND r.reviewer_type = 'user' - AND COALESCE(f.include_in_rating, true) - AND r.helpfulness_weight > 0 - ), - overall AS ( - SELECT round(COALESCE(sum(w), 0), 2) AS weight_sum, - CASE WHEN sum(w) > 0 THEN round((3 * 4.2 + sum(w * star)) / (3 + sum(w)), 2) END AS score - FROM eligible - WHERE star BETWEEN 1 AND 5 - ), - per_criterion AS ( - SELECT c.key, - count(*) AS writers, - round(sum(e.w), 2) AS weight_sum, - round((3 * 4.2 + sum(e.w * (c.value #>> '{}')::numeric)) / (3 + sum(e.w)), 2) AS score - FROM eligible e - CROSS JOIN LATERAL jsonb_each( - CASE WHEN jsonb_typeof(e.criteria_ratings) = 'object' THEN e.criteria_ratings - ELSE '{}'::jsonb END - ) AS c(key, value) - WHERE jsonb_typeof(c.value) = 'number' - AND (c.value #>> '{}')::numeric BETWEEN 1 AND 5 - GROUP BY c.key - ) - UPDATE rate.master_products m - SET rating_score = o.score, - rating_weight_sum = o.weight_sum, - criteria_scores = COALESCE( - (SELECT jsonb_object_agg(pc.key, jsonb_build_object( - 'score', pc.score, 'weight_sum', pc.weight_sum, 'writers', pc.writers)) - FROM per_criterion pc - WHERE pc.writers >= 3 AND pc.weight_sum >= 1.5), - '{}'::jsonb) - FROM overall o - WHERE m.id = p_master_id; -$$; +DECLARE + v_item uuid := rate.master_rating_item(p_master_id); + v_score numeric; + v_weight numeric; + v_criteria jsonb; +BEGIN + SELECT x.rating_score, x.rating_weight_sum, x.criteria_scores + INTO v_score, v_weight, v_criteria + FROM rate.master_rating_extras(p_master_id) x; + EXECUTE format($q$ + UPDATE rate.master_products m + SET rating_score = $2, rating_weight_sum = $3, criteria_scores = $4 + WHERE %s = $1 + AND (m.rating_score, m.rating_weight_sum, m.criteria_scores) IS DISTINCT FROM ($2, $3, $4) + $q$, CASE WHEN v_item IS NULL THEN 'm.id' ELSE 'm.item_id' END) + USING COALESCE(v_item, p_master_id), v_score, v_weight, v_criteria; +END $$; +REVOKE EXECUTE ON FUNCTION rate.master_rating_item(uuid) FROM PUBLIC, anon, authenticated; +REVOKE EXECUTE ON FUNCTION rate.master_rating_extras(uuid) FROM PUBLIC, anon, authenticated; REVOKE EXECUTE ON FUNCTION rate.refresh_master_rating_extras(uuid) FROM PUBLIC, anon, authenticated; -GRANT EXECUTE ON FUNCTION rate.refresh_master_rating_extras(uuid) TO service_role, rate_worker; +GRANT EXECUTE ON FUNCTION rate.master_rating_item(uuid) TO service_role; +GRANT EXECUTE ON FUNCTION rate.master_rating_extras(uuid) TO service_role; +GRANT EXECUTE ON FUNCTION rate.refresh_master_rating_extras(uuid) TO service_role; --- 7. Wrap the live refresh --------------------------------------------------------------------- +-- 7. Keep them current: triggers on what they read -------------------------------------------- -- --- Renaming keeps the live body, its owner, SECURITY DEFINER and grants, whatever version it --- is. Only on the first run: a re-run finds the base and leaves it. +-- No refresh function is replaced or renamed, so a later CREATE OR REPLACE of +-- refresh_master_product_rating or refresh_item_rating (M1+M2, its rollback, a hotfix) cannot +-- drop these columns' updates. The triggers fire on: +-- rate.reviews every insert, update and delete (star, exact star, criteria, weight) +-- rate.review_flags every insert, update and delete (include_in_rating) +-- rate.master_products a change of user_rating_weighted or authentic_review_count, which the +-- live refresh writes after a listing moves between masters or Items +-- Statement-level with transition tables for the two review tables, so a bulk import refreshes +-- each touched master once per statement. -DO $$ +CREATE OR REPLACE FUNCTION rate.refresh_rating_extras_of_masters(p_masters uuid[]) +RETURNS void +LANGUAGE plpgsql +SECURITY DEFINER +SET search_path = '' +AS $$ +DECLARE + v_master uuid; +BEGIN + FOR v_master IN SELECT DISTINCT unnest(p_masters) ORDER BY 1 LOOP + IF v_master IS NOT NULL THEN + PERFORM rate.refresh_master_rating_extras(v_master); + END IF; + END LOOP; +END $$; + +CREATE OR REPLACE FUNCTION rate.reviews_refresh_rating_extras() +RETURNS trigger +LANGUAGE plpgsql +SECURITY DEFINER +SET search_path = '' +AS $$ BEGIN - IF to_regprocedure('rate.refresh_master_product_rating_base(uuid)') IS NULL THEN - ALTER FUNCTION rate.refresh_master_product_rating(uuid) - RENAME TO refresh_master_product_rating_base; + IF TG_OP = 'INSERT' THEN + PERFORM rate.refresh_rating_extras_of_masters(ARRAY( + SELECT p.cluster_id FROM changed_new r JOIN rate.products p ON p.id = r.product_id)); + ELSIF TG_OP = 'UPDATE' THEN + PERFORM rate.refresh_rating_extras_of_masters(ARRAY( + SELECT p.cluster_id FROM (SELECT product_id FROM changed_new + UNION SELECT product_id FROM changed_old) r + JOIN rate.products p ON p.id = r.product_id)); + ELSE + PERFORM rate.refresh_rating_extras_of_masters(ARRAY( + SELECT p.cluster_id FROM changed_old r JOIN rate.products p ON p.id = r.product_id)); END IF; + RETURN NULL; END $$; -CREATE OR REPLACE FUNCTION rate.refresh_master_product_rating(p_master_id uuid) -RETURNS void +CREATE OR REPLACE FUNCTION rate.review_flags_refresh_rating_extras() +RETURNS trigger LANGUAGE plpgsql SECURITY DEFINER SET search_path = '' AS $$ BEGIN - PERFORM rate.refresh_master_product_rating_base(p_master_id); - PERFORM rate.refresh_master_rating_extras(p_master_id); -END; -$$; + IF TG_OP = 'INSERT' THEN + PERFORM rate.refresh_rating_extras_of_masters(ARRAY( + SELECT p.cluster_id FROM changed_new f + JOIN rate.reviews r ON r.id = f.review_id JOIN rate.products p ON p.id = r.product_id)); + ELSIF TG_OP = 'UPDATE' THEN + PERFORM rate.refresh_rating_extras_of_masters(ARRAY( + SELECT p.cluster_id FROM (SELECT review_id FROM changed_new + UNION SELECT review_id FROM changed_old) f + JOIN rate.reviews r ON r.id = f.review_id JOIN rate.products p ON p.id = r.product_id)); + ELSE + -- A flag deleted with its review (ON DELETE CASCADE) finds no review here; the reviews + -- trigger refreshes that master. + PERFORM rate.refresh_rating_extras_of_masters(ARRAY( + SELECT p.cluster_id FROM changed_old f + JOIN rate.reviews r ON r.id = f.review_id JOIN rate.products p ON p.id = r.product_id)); + END IF; + RETURN NULL; +END $$; -REVOKE EXECUTE ON FUNCTION rate.refresh_master_product_rating(uuid) FROM PUBLIC, anon, authenticated; -GRANT EXECUTE ON FUNCTION rate.refresh_master_product_rating(uuid) TO service_role, rate_worker; -REVOKE EXECUTE ON FUNCTION rate.refresh_master_product_rating_base(uuid) FROM PUBLIC, anon, authenticated; +CREATE OR REPLACE FUNCTION rate.master_products_refresh_rating_extras() +RETURNS trigger +LANGUAGE plpgsql +SECURITY DEFINER +SET search_path = '' +AS $$ +BEGIN + PERFORM rate.refresh_master_rating_extras(NEW.id); + RETURN NULL; +END $$; + +REVOKE EXECUTE ON FUNCTION rate.refresh_rating_extras_of_masters(uuid[]) FROM PUBLIC, anon, authenticated; +REVOKE EXECUTE ON FUNCTION rate.reviews_refresh_rating_extras() FROM PUBLIC, anon, authenticated; +REVOKE EXECUTE ON FUNCTION rate.review_flags_refresh_rating_extras() FROM PUBLIC, anon, authenticated; +REVOKE EXECUTE ON FUNCTION rate.master_products_refresh_rating_extras() FROM PUBLIC, anon, authenticated; --- 8. Backfill: every master gets its rating_score and criteria_scores once. Re-runnable. +DROP TRIGGER IF EXISTS trg_reviews_rating_extras_ins ON rate.reviews; +CREATE TRIGGER trg_reviews_rating_extras_ins AFTER INSERT ON rate.reviews + REFERENCING NEW TABLE AS changed_new + FOR EACH STATEMENT EXECUTE FUNCTION rate.reviews_refresh_rating_extras(); +DROP TRIGGER IF EXISTS trg_reviews_rating_extras_upd ON rate.reviews; +CREATE TRIGGER trg_reviews_rating_extras_upd AFTER UPDATE ON rate.reviews + REFERENCING OLD TABLE AS changed_old NEW TABLE AS changed_new + FOR EACH STATEMENT EXECUTE FUNCTION rate.reviews_refresh_rating_extras(); +DROP TRIGGER IF EXISTS trg_reviews_rating_extras_del ON rate.reviews; +CREATE TRIGGER trg_reviews_rating_extras_del AFTER DELETE ON rate.reviews + REFERENCING OLD TABLE AS changed_old + FOR EACH STATEMENT EXECUTE FUNCTION rate.reviews_refresh_rating_extras(); -SELECT rate.refresh_master_rating_extras(id) FROM rate.master_products; +DROP TRIGGER IF EXISTS trg_review_flags_rating_extras_ins ON rate.review_flags; +CREATE TRIGGER trg_review_flags_rating_extras_ins AFTER INSERT ON rate.review_flags + REFERENCING NEW TABLE AS changed_new + FOR EACH STATEMENT EXECUTE FUNCTION rate.review_flags_refresh_rating_extras(); +DROP TRIGGER IF EXISTS trg_review_flags_rating_extras_upd ON rate.review_flags; +CREATE TRIGGER trg_review_flags_rating_extras_upd AFTER UPDATE ON rate.review_flags + REFERENCING OLD TABLE AS changed_old NEW TABLE AS changed_new + FOR EACH STATEMENT EXECUTE FUNCTION rate.review_flags_refresh_rating_extras(); +DROP TRIGGER IF EXISTS trg_review_flags_rating_extras_del ON rate.review_flags; +CREATE TRIGGER trg_review_flags_rating_extras_del AFTER DELETE ON rate.review_flags + REFERENCING OLD TABLE AS changed_old + FOR EACH STATEMENT EXECUTE FUNCTION rate.review_flags_refresh_rating_extras(); +DROP TRIGGER IF EXISTS trg_master_products_rating_extras ON rate.master_products; +CREATE TRIGGER trg_master_products_rating_extras + AFTER UPDATE OF user_rating_weighted, authentic_review_count ON rate.master_products + FOR EACH ROW + WHEN ((OLD.user_rating_weighted, OLD.authentic_review_count) + IS DISTINCT FROM (NEW.user_rating_weighted, NEW.authentic_review_count)) + EXECUTE FUNCTION rate.master_products_refresh_rating_extras(); + +COMMIT; + +-- 8. Backfill, in its own transaction after the schema change commits, so the ALTER TABLE +-- locks are already released and only the touched master rows are locked. Masters with no +-- counted review already hold the right values (NULL, 0, {}) from the column defaults. +-- Re-runnable; it writes only rows whose values differ. A catalog of many thousand reviewed +-- masters can run it in id ranges instead. + +BEGIN; +SELECT rate.refresh_master_rating_extras(m.id) +FROM rate.master_products m +WHERE m.authentic_review_count > 0 OR m.user_rating_weighted > 0 +ORDER BY m.id; COMMIT; diff --git a/scripts/test-criteria-ratings-proposal.mjs b/scripts/test-criteria-ratings-proposal.mjs index 064c1a59d..ba22d1608 100644 --- a/scripts/test-criteria-ratings-proposal.mjs +++ b/scripts/test-criteria-ratings-proposal.mjs @@ -1,9 +1,10 @@ #!/usr/bin/env node // Replays the DOS-Me criteria ratings PROPOSAL (DOS/Rate#1543, phase P2 of #1531) on the -// migration replay stack: a small catalog with criteria reviews, the proposal twice, the -// rating and criteria scores it computes, the refresh wrapper as the worker calls it, every -// verification query, the grants, the rollback twice, then the same on top of M0 and M1+M2 -// (whose Item-aware refresh the wrapper must keep). Nothing touches a real database. +// migration replay stack: a small catalog with criteria reviews, the proposal twice, the rating +// and criteria scores its triggers keep current (review writes, a weight change, a flag that +// leaves the rating), every verification query, the grants, the rollback twice; then the +// proposal BEFORE M0 and M1+M2 (an Item pools its variants' reviews), the M1+M2 rollback under +// it, and the proposal applied again AFTER M1+M2. Nothing touches a real database. // // Usage: node scripts/test-criteria-ratings-proposal.mjs (needs Docker) @@ -21,6 +22,7 @@ const rollback = read('2026-10-31-dos-me-criteria-ratings-rollback.sql'); const verify = read('2026-10-31-dos-me-criteria-ratings-verify.sql'); const m0 = read('2026-10-29-dos-me-m0-seed-rekey.sql'); const m1m2 = read('2026-10-28-dos-me-m1-m2-items.sql'); +const m1m2Rollback = read('2026-10-28-dos-me-m1-m2-items-rollback.sql'); const container = `rate-criteria-proposal-${process.pid}`; const removeContainer = () => @@ -43,17 +45,28 @@ assert.equal(checks.length, 6, 'the verification lists its expect-0 queries'); const PHONES = 'c0000000-0000-4000-8000-000000000001'; const MERCHANT = 'e0000000-0000-4000-8000-000000000001'; -const rated = 'a0000000-0000-4000-8000-000000000001'; -const empty = 'a0000000-0000-4000-8000-000000000002'; -const listing = 'b0000000-0000-4000-8000-000000000001'; +const x128 = 'a0000000-0000-4000-8000-000000000128'; +const x256 = 'a0000000-0000-4000-8000-000000000256'; +const empty = 'a0000000-0000-4000-8000-0000000000ee'; +const listing128 = 'b0000000-0000-4000-8000-000000000128'; +const listing256 = 'b0000000-0000-4000-8000-000000000256'; const review = (n) => `d0000000-0000-4000-8000-00000000000${n}`; try { const { sql, asRole } = startRateDb({ name: container }); const checkAll = (when) => { for (const { id, query } of checks) - assert.equal(sql(query), '0', `${id} after ${when}`); + assert.equal(sql(query), '0', `${id} ${when}`); }; + const row = (id) => + sql( + `SELECT coalesce(rating_score::text, '-') || '|' || rating_weight_sum || '|' || criteria_scores::text + FROM rate.master_products WHERE id = '${id}'` + ); + const weighted = (id) => + sql( + `SELECT user_rating_weighted FROM rate.master_products WHERE id = '${id}'` + ); sql(` INSERT INTO rate.categories (id, name, slug) VALUES ('${PHONES}', 'Điện thoại', 'smartphones'); @@ -62,89 +75,118 @@ try { INSERT INTO rate.master_products (id, brand, model, model_key, name, slug, category_id, status, storage_gb, min_price, max_price) VALUES - ('${rated}', 'Acme', 'Phone X', 'acme:phone-x:128gb', 'Acme Phone X 128GB', + ('${x128}', 'Acme', 'Phone X', 'acme:phone-x:128gb', 'Acme Phone X 128GB', 'acme-phone-x-128gb', '${PHONES}', 'published', 128, 10000000, 10000000), + ('${x256}', 'Acme', 'Phone X', 'acme:phone-x:256gb', 'Acme Phone X 256GB', + 'acme-phone-x-256gb', '${PHONES}', 'published', 256, 12000000, 12000000), ('${empty}', 'Acme', 'Phone Y', 'acme:phone-y:128gb', 'Acme Phone Y 128GB', 'acme-phone-y-128gb', '${PHONES}', 'published', 128, 9000000, 9000000); INSERT INTO rate.products (id, merchant_id, platform, platform_item_id, title, price_current, url, cluster_id) - VALUES ('${listing}', '${MERCHANT}', 'shopee', 'x1', 'Acme Phone X 128GB', 10000000, - 'https://shopee.vn/x1', '${rated}'); - -- Three criteria reviews (one with its exact 0.1 star), one excluded from the rating, - -- one marketplace review with no criteria and weight 2. + VALUES + ('${listing128}', '${MERCHANT}', 'shopee', 'x128', 'Acme Phone X 128GB', 10000000, + 'https://shopee.vn/x128', '${x128}'), + ('${listing256}', '${MERCHANT}', 'shopee', 'x256', 'Acme Phone X 256GB', 12000000, + 'https://shopee.vn/x256', '${x256}'); + -- On the 128GB: three criteria reviews, one excluded from the rating, one marketplace + -- review with no criteria and weight 2. On the 256GB: one criteria review. INSERT INTO rate.reviews (id, product_id, rating_star, comment, reviewer_type, criteria_ratings) VALUES - ('${review(1)}', '${listing}', 5, 'Màn hình đẹp, pin ổn.', 'user', '{"display": 5, "value": 4}'), - ('${review(2)}', '${listing}', 4, 'Dùng tốt trong tầm giá.', 'user', '{"display": 4, "value": 4}'), - ('${review(3)}', '${listing}', 3, 'Màn hơi tối ngoài nắng.', 'user', '{"display": 3}'), - ('${review(4)}', '${listing}', 1, 'Giao sai màu.', 'user', '{"display": 1, "value": 1}'), - ('${review(5)}', '${listing}', 5, 'Máy chạy mượt.', 'user', '{}'); + ('${review(1)}', '${listing128}', 5, 'Màn hình đẹp, pin ổn.', 'user', + '{"display": 5, "value": 4, "Bad Key!": 5, "camera": "5"}'), + ('${review(2)}', '${listing128}', 4, 'Dùng tốt trong tầm giá.', 'user', '{"display": 4, "value": 4}'), + ('${review(3)}', '${listing128}', 3, 'Màn hơi tối ngoài nắng.', 'user', '{"display": 3}'), + ('${review(4)}', '${listing128}', 1, 'Giao sai màu.', 'user', '{"display": 1, "value": 1}'), + ('${review(5)}', '${listing128}', 5, 'Máy chạy mượt.', 'user', '{}'), + ('${review(6)}', '${listing256}', 2, 'Màn ám vàng.', 'user', '{"display": 2}'); INSERT INTO rate.review_flags (review_id, verdict, include_in_rating, detector, detector_version, input_hash) VALUES ('${review(1)}', 'authentic', true, 'replay', 'v1', 'h1'), ('${review(2)}', 'authentic', true, 'replay', 'v1', 'h2'), ('${review(3)}', 'authentic', true, 'replay', 'v1', 'h3'), ('${review(4)}', 'suspicious', false, 'replay', 'v1', 'h4'), - ('${review(5)}', 'authentic', true, 'replay', 'v1', 'h5'); - UPDATE rate.reviews SET helpfulness_weight = 1 - WHERE id IN ('${review(1)}', '${review(2)}', '${review(3)}', '${review(4)}'); + ('${review(5)}', 'authentic', true, 'replay', 'v1', 'h5'), + ('${review(6)}', 'authentic', true, 'replay', 'v1', 'h6'); + UPDATE rate.reviews SET helpfulness_weight = 1 WHERE id <> '${review(5)}'; UPDATE rate.reviews SET helpfulness_weight = 2 WHERE id = '${review(5)}'; - SELECT rate.refresh_master_product_rating('${rated}'); + SELECT rate.refresh_master_product_rating('${x128}'); + SELECT rate.refresh_master_product_rating('${x256}'); `); - const masterRow = (id) => - sql( - `SELECT coalesce(rating_score::text, '-') || '|' || rating_weight_sum || '|' || criteria_scores::text - FROM rate.master_products WHERE id = '${id}'` - ); - // Weights 1 + 1 + 1 + 2 = 5 (review 4 is excluded); stars 5, 4.3 (exact), 3, 5. - // (3 * 4.2 + 5 + 4.3 + 3 + 10) / (3 + 5) = 34.9 / 8 = 4.36. - // display: 3 writers, weight 3: (12.6 + 5 + 4 + 3) / 6 = 4.10. value: 2 writers, hidden. - const expected = + // Per master (no Items yet). 128GB: weights 1 + 1 + 1 + 2 = 5 (review 4 is excluded), stars + // 5, 4.3 (exact), 3, 5: (3 * 4.2 + 22.3) / 8 = 4.36. display: 3 writers, weight 3: + // (12.6 + 12) / 6 = 4.10. value has 2 writers (hidden); a bad key and a string star are not + // criteria. 256GB: (12.6 + 2) / 4 = 3.65, one writer, no criteria. + const per128 = '4.36|5.00|{"display": {"score": 4.10, "writers": 3, "weight_sum": 3.00}}'; + const per256 = '3.65|1.00|{}'; - const applyAndCheck = (when) => { + const applyAndCheckPerMaster = (when) => { sql(proposal); sql(proposal); - sql(`UPDATE rate.reviews SET rating_exact = 4.3 WHERE id = '${review(2)}'`); - // The worker's usual call goes through the wrapper: the live body, then the extras. + // A criteria review gets its exact star: the reviews trigger refreshes the master. asRole( 'rate_worker', - `SELECT rate.refresh_master_product_rating('${rated}')` + `UPDATE rate.reviews SET rating_exact = 4.3 WHERE id = '${review(2)}'` ); - assert.equal(masterRow(rated), expected, `rating and criteria ${when}`); + assert.equal(row(x128), per128, `128GB ${when}`); + assert.equal(row(x256), per256, `256GB ${when}`); + assert.equal(row(empty), '-|0.00|{}', `a master with no reviews ${when}`); assert.equal( - masterRow(empty), - '-|0.00|{}', - `a master with no reviews ${when}` + weighted(x128), + '4.40', + `user_rating_weighted untouched ${when}` + ); + checkAll(`after applying ${when}`); + + // A flag that takes a review out of the rating refreshes it, and back. + sql( + `UPDATE rate.review_flags SET include_in_rating = false WHERE review_id = '${review(3)}'` ); assert.equal( - sql( - `SELECT user_rating_weighted FROM rate.master_products WHERE id = '${rated}'` - ), - '4.40', - `the live body still fills user_rating_weighted ${when}` + row(x128), + '4.56|4.00|{}', + `review 3 out of the rating ${when}` ); - checkAll(when); + sql( + `UPDATE rate.review_flags SET include_in_rating = true WHERE review_id = '${review(3)}'` + ); + assert.equal(row(x128), per128, `review 3 back ${when}`); + for (const fn of [ - `rate.refresh_master_rating_extras('${rated}')`, - `rate.refresh_master_product_rating('${rated}')`, - `rate.refresh_master_product_rating_base('${rated}')`, - ]) + `rate.refresh_master_rating_extras('${x128}')`, + `rate.master_rating_extras('${x128}')`, + ]) { assert.throws( - () => asRole('anon', `SELECT ${fn}`), + () => asRole('anon', `SELECT * FROM ${fn}`), /permission denied/, `anon ${fn}` ); + assert.throws( + () => asRole('rate_worker', `SELECT * FROM ${fn}`), + /permission denied/, + `worker ${fn}` + ); + } assert.equal( - asRole('anon', `SELECT count(*) FROM rate.review_points`), + asRole('anon', 'SELECT count(*) FROM rate.review_points'), '0', `anon reads review_points ${when}` ); + for (const role of ['anon', 'authenticated', 'rate_worker']) + assert.throws( + () => + asRole( + role, + `INSERT INTO rate.review_points (review_id, kind, body) VALUES ('${review(1)}', 'pro', 'Pin trâu')` + ), + /permission denied|row-level security/, + `${role} cannot write review_points ${when}` + ); assert.throws( () => asRole( 'anon', - `INSERT INTO rate.product_points (master_id, kind, label) VALUES ('${rated}', 'pro', 'Pin tốt')` + `INSERT INTO rate.product_points (master_id, kind, label) VALUES ('${x128}', 'pro', 'Pin tốt')` ), /permission denied|row-level security/, `anon cannot write product_points ${when}` @@ -152,7 +194,7 @@ try { asRole( 'rate_worker', `INSERT INTO rate.product_points (master_id, kind, label, support, reviewer_count) - VALUES ('${rated}', 'pro', 'Màn hình đẹp', 2, 2)` + VALUES ('${x128}', 'pro', 'Màn hình đẹp', 2, 2)` ); assert.throws( () => @@ -162,6 +204,19 @@ try { /review_points_body_check/, `a pro line under 5 characters is refused ${when}` ); + // A weight above 3 counts as 3: review 5 at weight 9 equals review 5 at weight 3. + sql( + `UPDATE rate.reviews SET helpfulness_weight = 9 WHERE id = '${review(5)}'` + ); + const capped = row(x128); + sql( + `UPDATE rate.reviews SET helpfulness_weight = 3 WHERE id = '${review(5)}'` + ); + assert.equal(row(x128), capped, `weight capped at 3 ${when}`); + sql( + `UPDATE rate.reviews SET helpfulness_weight = 2 WHERE id = '${review(5)}'` + ); + assert.equal(row(x128), per128, `weight restored ${when}`); }; const rollBackAndCheck = (when) => { @@ -169,41 +224,95 @@ try { sql(rollback); assert.equal( sql(`SELECT count(*) FROM information_schema.columns - WHERE table_schema = 'rate' AND column_name IN ('rating_exact', 'rating_score', 'criteria_scores')`), + WHERE table_schema = 'rate' + AND column_name IN ('rating_exact', 'rating_score', 'criteria_scores')`), '0', `columns dropped ${when}` ); + assert.equal( + sql( + `SELECT count(*) FROM pg_trigger WHERE tgname LIKE 'trg_%rating_extras%'` + ), + '0', + `triggers dropped ${when}` + ); assert.equal( sql(`SELECT to_regclass('rate.review_points') IS NULL AND to_regclass('rate.product_points') IS NULL - AND to_regprocedure('rate.refresh_master_product_rating_base(uuid)') IS NULL`), + AND to_regprocedure('rate.refresh_master_rating_extras(uuid)') IS NULL`), 't', - `tables and the base name gone ${when}` + `tables and functions dropped ${when}` ); asRole( 'rate_worker', - `SELECT rate.refresh_master_product_rating('${rated}')` - ); - assert.equal( - sql( - `SELECT user_rating_weighted FROM rate.master_products WHERE id = '${rated}'` - ), - '4.40', - `the restored refresh still works ${when}` + `SELECT rate.refresh_master_product_rating('${x128}')` ); + sql(`INSERT INTO rate.reviews (id, product_id, rating_star, comment, reviewer_type) + VALUES ('d0000000-0000-4000-8000-0000000000ff', '${listing256}', 4, 'Ổn.', 'user'); + DELETE FROM rate.reviews WHERE id = 'd0000000-0000-4000-8000-0000000000ff';`); }; - applyAndCheck('on the reference stack'); + // 1. On the reference stack. + applyAndCheckPerMaster('on the reference stack'); rollBackAndCheck('on the reference stack'); - // The order JOY applies them in: M0, M1+M2 (an Item-aware refresh), then this proposal. + // 2. The proposal first, then M0 and M1+M2: the two sizes become one Item, and both + // variants show the Item's pooled rating. Weights 5 + 1 = 6, stars 22.3 + 2: + // (12.6 + 24.3) / 9 = 4.10. display: 4 writers, weight 4: (12.6 + 14) / 7 = 3.80. + applyAndCheckPerMaster('before M1+M2'); sql(m0); sql(m1m2); - applyAndCheck('after M0 and M1+M2'); - rollBackAndCheck('after M0 and M1+M2'); - // A full re-apply after a rollback. - applyAndCheck('re-applied after a rollback'); + const pooled = + '4.10|6.00|{"display": {"score": 3.80, "writers": 4, "weight_sum": 4.00}}'; + assert.equal( + sql( + `SELECT count(DISTINCT item_id) FROM rate.master_products WHERE id IN ('${x128}', '${x256}')` + ), + '1', + 'the two sizes share one Item' + ); + assert.equal(row(x128), pooled, '128GB shows the Item rating'); + assert.equal(row(x256), pooled, '256GB shows the Item rating'); + checkAll('after M1+M2 on top'); + // A vote's weight change on the Item path (refresh_item_rating, not the master refresh): + // review 5 at weight 1: weights 5, stars 5 + 4.3 + 3 + 5 + 2 = 19.3, (12.6 + 19.3) / 8 = 3.99. + sql( + `UPDATE rate.reviews SET helpfulness_weight = 1 WHERE id = '${review(5)}'` + ); + asRole('rate_worker', `SELECT rate.refresh_master_product_rating('${x256}')`); + const pooledAfterVote = + '3.99|5.00|{"display": {"score": 3.80, "writers": 4, "weight_sum": 4.00}}'; + assert.equal(row(x128), pooledAfterVote, '128GB after the vote'); + assert.equal(row(x256), pooledAfterVote, '256GB after the vote'); + checkAll('after a vote on the Item path'); + + // 3. The M1+M2 rollback under the proposal: the triggers stay and the 128GB, whose review + // changes, goes back to its own rating. The 256GB keeps the pooled values until something + // touches it: V6 shows it, and the runbook's step after an M1+M2 rollback (the backfill of + // section 8 again) brings it back. + sql(m1m2Rollback); + sql( + `UPDATE rate.reviews SET helpfulness_weight = 2 WHERE id = '${review(5)}'` + ); + assert.equal(row(x128), per128, '128GB after the M1+M2 rollback'); + const stale = checks.find(({ id }) => id === 'V6'); + assert.equal(sql(stale.query), '1', 'V6 shows the 256GB as out of date'); + const backfill = proposal.slice(proposal.lastIndexOf('BEGIN;')); + assert.match(backfill, /refresh_master_rating_extras/, 'the backfill step'); + sql(backfill); + assert.equal(row(x256), per256, '256GB after the M1+M2 rollback'); + checkAll('after the M1+M2 rollback'); + + // 4. M1+M2 again, then the proposal applied (again) after it: same pooled values. + sql(m1m2); + sql(proposal); + assert.equal(row(x128), pooled, '128GB, proposal after M1+M2'); + assert.equal(row(x256), pooled, '256GB, proposal after M1+M2'); + checkAll('with the proposal applied after M1+M2'); + rollBackAndCheck('after M1+M2'); - console.log('criteria ratings proposal: apply, verify and rollback replayed'); + console.log( + 'criteria ratings proposal: apply, triggers, Items, verify and rollback replayed' + ); } finally { removeContainer(); } From 4b12a3b7520c4d1ee2e30ae2477d3c8ae9b292a7 Mon Sep 17 00:00:00 2001 From: JOY <5027251+JOY@users.noreply.github.com> Date: Sun, 1 Nov 2026 02:02:44 +0700 Subject: [PATCH 3/5] fix(proposals): no sibling deadlock and no-variant listings in criteria ratings Round 2 review of #1549. Variants of an Item are written in id order with SKIP LOCKED, as refresh_item_rating does, so two refreshes of sibling variants never deadlock. Triggers resolve masters from listings, so a review on an Item listing with no variant refreshes the Item, and each Item is refreshed once per statement. Only changed rating inputs fire a refresh. The CHECKs are added NOT VALID and validated in the backfill transaction; V1 covers product_points. The replay adds a no-variant review, an unchanged rewrite and its delete; the runbook lists bulk flag imports, concurrent writers and listing deletes as known limits. Refs #1543 Co-Authored-By: Claude Opus 5.5 --- ...10-31-dos-me-criteria-ratings-rollback.sql | 2 +- ...6-10-31-dos-me-criteria-ratings-verify.sql | 7 +- .../2026-10-31-dos-me-criteria-ratings.md | 10 +- .../2026-10-31-dos-me-criteria-ratings.sql | 120 +++++++++++++----- scripts/test-criteria-ratings-proposal.mjs | 35 +++++ 5 files changed, 132 insertions(+), 42 deletions(-) diff --git a/Docs/Proposals/2026-10-31-dos-me-criteria-ratings-rollback.sql b/Docs/Proposals/2026-10-31-dos-me-criteria-ratings-rollback.sql index 74b1e06d8..2e84748f6 100644 --- a/Docs/Proposals/2026-10-31-dos-me-criteria-ratings-rollback.sql +++ b/Docs/Proposals/2026-10-31-dos-me-criteria-ratings-rollback.sql @@ -21,7 +21,7 @@ DROP TRIGGER IF EXISTS trg_master_products_rating_extras ON rate.master_products DROP FUNCTION IF EXISTS rate.master_products_refresh_rating_extras(); DROP FUNCTION IF EXISTS rate.review_flags_refresh_rating_extras(); DROP FUNCTION IF EXISTS rate.reviews_refresh_rating_extras(); -DROP FUNCTION IF EXISTS rate.refresh_rating_extras_of_masters(uuid[]); +DROP FUNCTION IF EXISTS rate.refresh_rating_extras_of_products(uuid[]); DROP FUNCTION IF EXISTS rate.refresh_master_rating_extras(uuid); DROP FUNCTION IF EXISTS rate.master_rating_extras(uuid); DROP FUNCTION IF EXISTS rate.master_rating_item(uuid); diff --git a/Docs/Proposals/2026-10-31-dos-me-criteria-ratings-verify.sql b/Docs/Proposals/2026-10-31-dos-me-criteria-ratings-verify.sql index 84dcaf9b5..9fe8ff517 100644 --- a/Docs/Proposals/2026-10-31-dos-me-criteria-ratings-verify.sql +++ b/Docs/Proposals/2026-10-31-dos-me-criteria-ratings-verify.sql @@ -3,14 +3,15 @@ -- stack. -- V1 Every new column exists. expect 0 -SELECT 9 - count(*) AS missing_columns +SELECT 10 - count(*) AS missing_columns FROM information_schema.columns WHERE table_schema = 'rate' AND ((table_name = 'reviews' AND column_name IN ('rating_exact', 'verdict', 'criteria_version', 'value_score', 'value_breakdown')) OR (table_name = 'master_products' AND column_name IN ('rating_score', 'rating_weight_sum', 'criteria_scores')) - OR (table_name = 'review_points' AND column_name = 'body')); + OR (table_name = 'review_points' AND column_name = 'body') + OR (table_name = 'product_points' AND column_name = 'label')); -- V2 Both new tables have row level security on. expect 0 SELECT 2 - count(*) AS tables_without_rls @@ -29,7 +30,7 @@ SELECT count(*) FILTER (WHERE to_regprocedure(w.signature) IS NULL) AS missing_or_open_functions FROM (VALUES ('rate.master_rating_item(uuid)'), ('rate.master_rating_extras(uuid)'), ('rate.refresh_master_rating_extras(uuid)'), - ('rate.refresh_rating_extras_of_masters(uuid[])'), + ('rate.refresh_rating_extras_of_products(uuid[])'), ('rate.reviews_refresh_rating_extras()'), ('rate.review_flags_refresh_rating_extras()'), ('rate.master_products_refresh_rating_extras()')) w(signature); diff --git a/Docs/Proposals/2026-10-31-dos-me-criteria-ratings.md b/Docs/Proposals/2026-10-31-dos-me-criteria-ratings.md index fa262a0da..f6acbbf3c 100644 --- a/Docs/Proposals/2026-10-31-dos-me-criteria-ratings.md +++ b/Docs/Proposals/2026-10-31-dos-me-criteria-ratings.md @@ -50,7 +50,7 @@ Design: [`2026-10-31-criteria-stars-pros-cons.html`](./2026-10-31-criteria-stars - `rate.refresh_master_rating_extras(master)` writes them, only where they changed. - Only the service role can run them. - **Triggers keep the values current.** - - On `rate.reviews` and `rate.review_flags`: every insert, update and delete. They run once per statement, so a bulk import refreshes each touched master once. + - On `rate.reviews` (a change of a rating input: star, exact star, criteria, weight, type, origin or listing) and `rate.review_flags` (`include_in_rating`), for every insert, update and delete. They run once per statement; see Known limits for bulk flag imports. - On `rate.master_products`: a change of `user_rating_weighted` or `authentic_review_count`, for example after a listing moves. - **No existing function is replaced or renamed.** A later `CREATE OR REPLACE` of a refresh function (M1+M2, its rollback, a hotfix) cannot stop these updates. @@ -58,8 +58,8 @@ Design: [`2026-10-31-criteria-stars-pros-cons.html`](./2026-10-31-criteria-stars 1. **Apply order:** before or after M0 and M1+M2. The replay covers both orders and the M1+M2 rollback. 2. **Apply on preview:** - - The schema transaction briefly takes ACCESS EXCLUSIVE locks on `rate.reviews` and `rate.master_products` (the `ALTER TABLE` statements). - - The backfill runs afterwards in its own transaction and locks only reviewed master rows. + - The schema transaction takes ACCESS EXCLUSIVE locks on `rate.reviews` and `rate.master_products` only for the `ALTER TABLE` statements. The new CHECKs are added NOT VALID, so nothing is scanned under that lock. + - The backfill transaction validates the CHECKs (a scan that lets reads and writes go on) and refreshes the reviewed masters, locking only those rows. - Run the verify file: V1 to V6 must return 0. - Compare V7: `rating_score` sits a little below `user_rating_weighted` for masters with few reviews, because of the prior. That is intended (D3). 3. **Then beta, then prod**, running the verify file after each. @@ -77,6 +77,10 @@ Roll back only while no phase reads or writes the new columns: before P3, P4 and ## Known limits +- **Bulk flag imports.** The existing `trg_review_flags_sync_weight` updates `rate.reviews` one row at a time. A bulk flag import therefore refreshes a master once per flag row, not once per import. Re-crawls that rewrite reviews unchanged refresh nothing: only changes to rating inputs count. +- **Concurrent writers.** A variant locked by another transaction is skipped, as `refresh_item_rating` does. This avoids deadlocks between sibling variants. The other transaction writes its own values when it reaches that variant. Like the live refresh, two writers on the same master can leave the older values until the next write; V6 shows any master out of date. +- **Deleting a listing with its reviews** leaves the master's values in place until its next review write or refresh, the same as `user_rating_weighted` today. V6 shows it. Re-run the backfill to fix it. + - **`rate_worker` can write the new columns directly.** It keeps its table-level UPDATE on `master_products` and on crawled reviews (20261018183000). The next trigger run overwrites such a write, and V6 shows it. Column-level grants are a separate hardening step. - **Per-category prior means:** version 1 uses 4.2 for every criterion. - **Not in this migration:** `submit_community_review` v2 (P3) and the value score weight (P6). Each comes with its own phase. diff --git a/Docs/Proposals/2026-10-31-dos-me-criteria-ratings.sql b/Docs/Proposals/2026-10-31-dos-me-criteria-ratings.sql index 5ed5a2962..b431fdaab 100644 --- a/Docs/Proposals/2026-10-31-dos-me-criteria-ratings.sql +++ b/Docs/Proposals/2026-10-31-dos-me-criteria-ratings.sql @@ -42,19 +42,19 @@ DO $$ BEGIN IF NOT EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'reviews_rating_exact_range') THEN ALTER TABLE rate.reviews ADD CONSTRAINT reviews_rating_exact_range - CHECK (rating_exact IS NULL OR (rating_exact >= 1 AND rating_exact <= 5)); + CHECK (rating_exact IS NULL OR (rating_exact >= 1 AND rating_exact <= 5)) NOT VALID; END IF; IF NOT EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'reviews_verdict_length') THEN ALTER TABLE rate.reviews ADD CONSTRAINT reviews_verdict_length - CHECK (verdict IS NULL OR char_length(verdict) <= 300); + CHECK (verdict IS NULL OR char_length(verdict) <= 300) NOT VALID; END IF; IF NOT EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'reviews_criteria_version_positive') THEN ALTER TABLE rate.reviews ADD CONSTRAINT reviews_criteria_version_positive - CHECK (criteria_version IS NULL OR criteria_version >= 1); + CHECK (criteria_version IS NULL OR criteria_version >= 1) NOT VALID; END IF; IF NOT EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'reviews_value_score_range') THEN ALTER TABLE rate.reviews ADD CONSTRAINT reviews_value_score_range - CHECK (value_score IS NULL OR (value_score >= 0 AND value_score <= 100)); + CHECK (value_score IS NULL OR (value_score >= 0 AND value_score <= 100)) NOT VALID; END IF; END $$; @@ -136,11 +136,11 @@ DO $$ BEGIN IF NOT EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'master_products_rating_score_range') THEN ALTER TABLE rate.master_products ADD CONSTRAINT master_products_rating_score_range - CHECK (rating_score IS NULL OR (rating_score >= 0 AND rating_score <= 5)); + CHECK (rating_score IS NULL OR (rating_score >= 0 AND rating_score <= 5)) NOT VALID; END IF; IF NOT EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'master_products_rating_weight_sum_nonneg') THEN ALTER TABLE rate.master_products ADD CONSTRAINT master_products_rating_weight_sum_nonneg - CHECK (rating_weight_sum >= 0); + CHECK (rating_weight_sum >= 0) NOT VALID; END IF; END $$; @@ -260,12 +260,20 @@ BEGIN SELECT x.rating_score, x.rating_weight_sum, x.criteria_scores INTO v_score, v_weight, v_criteria FROM rate.master_rating_extras(p_master_id) x; + -- Variants in id order, skipping one another transaction holds, as refresh_item_rating + -- does: that transaction is refreshing the same Item and writes when it gets there, so two + -- refreshes of sibling variants never wait on each other (no deadlock). EXECUTE format($q$ UPDATE rate.master_products m SET rating_score = $2, rating_weight_sum = $3, criteria_scores = $4 - WHERE %s = $1 - AND (m.rating_score, m.rating_weight_sum, m.criteria_scores) IS DISTINCT FROM ($2, $3, $4) - $q$, CASE WHEN v_item IS NULL THEN 'm.id' ELSE 'm.item_id' END) + WHERE m.id IN ( + SELECT mp.id FROM rate.master_products mp + WHERE %s = $1 + AND (mp.rating_score, mp.rating_weight_sum, mp.criteria_scores) + IS DISTINCT FROM ($2, $3, $4) + ORDER BY mp.id + FOR UPDATE SKIP LOCKED) + $q$, CASE WHEN v_item IS NULL THEN 'mp.id' ELSE 'mp.item_id' END) USING COALESCE(v_item, p_master_id), v_score, v_weight, v_criteria; END $$; @@ -285,25 +293,57 @@ GRANT EXECUTE ON FUNCTION rate.refresh_master_rating_extras(uuid) TO service_rol -- rate.review_flags every insert, update and delete (include_in_rating) -- rate.master_products a change of user_rating_weighted or authentic_review_count, which the -- live refresh writes after a listing moves between masters or Items --- Statement-level with transition tables for the two review tables, so a bulk import refreshes --- each touched master once per statement. - -CREATE OR REPLACE FUNCTION rate.refresh_rating_extras_of_masters(p_masters uuid[]) +-- Statement-level with transition tables for the two review tables, and only rows whose rating +-- inputs changed count, so a re-crawl that rewrites reviews unchanged refreshes nothing. A bulk +-- flag import still refreshes a master once per flag row: the existing trg_review_flags_sync_weight +-- updates rate.reviews one row at a time (20261014033000). + +-- The masters whose ratings a set of listings feeds: each listing's master, and once M1+M2 is +-- in, one variant of the Item of a listing with no variant (cluster_id NULL, item_id set). +-- Each Item is refreshed once, whichever of its variants comes first. +CREATE OR REPLACE FUNCTION rate.refresh_rating_extras_of_products(p_products uuid[]) RETURNS void LANGUAGE plpgsql SECURITY DEFINER SET search_path = '' AS $$ DECLARE + v_masters uuid[]; + v_extra uuid[]; v_master uuid; + v_item uuid; + v_done uuid[] := '{}'; BEGIN - FOR v_master IN SELECT DISTINCT unnest(p_masters) ORDER BY 1 LOOP - IF v_master IS NOT NULL THEN - PERFORM rate.refresh_master_rating_extras(v_master); + IF p_products IS NULL OR cardinality(p_products) = 0 THEN + RETURN; + END IF; + v_masters := ARRAY(SELECT DISTINCT p.cluster_id FROM rate.products p + WHERE p.id = ANY (p_products) AND p.cluster_id IS NOT NULL); + IF EXISTS (SELECT 1 FROM pg_catalog.pg_attribute + WHERE attrelid = 'rate.products'::pg_catalog.regclass + AND attname = 'item_id' AND NOT attisdropped) THEN + EXECUTE $q$ + SELECT ARRAY( + SELECT DISTINCT ON (p.item_id) m.id + FROM rate.products p + JOIN rate.master_products m ON m.item_id = p.item_id + WHERE p.id = ANY ($1) AND p.cluster_id IS NULL AND p.item_id IS NOT NULL + ORDER BY p.item_id, m.id) + $q$ INTO v_extra USING p_products; + v_masters := v_masters || v_extra; + END IF; + FOR v_master IN SELECT DISTINCT unnest(v_masters) ORDER BY 1 LOOP + v_item := rate.master_rating_item(v_master); + IF v_item IS NOT NULL THEN + CONTINUE WHEN v_item = ANY (v_done); + v_done := v_done || v_item; END IF; + PERFORM rate.refresh_master_rating_extras(v_master); END LOOP; END $$; +-- Only rows whose rating inputs changed count, so a re-crawl that rewrites a review unchanged +-- (ON CONFLICT DO UPDATE) refreshes nothing. CREATE OR REPLACE FUNCTION rate.reviews_refresh_rating_extras() RETURNS trigger LANGUAGE plpgsql @@ -312,16 +352,19 @@ SET search_path = '' AS $$ BEGIN IF TG_OP = 'INSERT' THEN - PERFORM rate.refresh_rating_extras_of_masters(ARRAY( - SELECT p.cluster_id FROM changed_new r JOIN rate.products p ON p.id = r.product_id)); + PERFORM rate.refresh_rating_extras_of_products(ARRAY(SELECT product_id FROM changed_new)); ELSIF TG_OP = 'UPDATE' THEN - PERFORM rate.refresh_rating_extras_of_masters(ARRAY( - SELECT p.cluster_id FROM (SELECT product_id FROM changed_new - UNION SELECT product_id FROM changed_old) r - JOIN rate.products p ON p.id = r.product_id)); + PERFORM rate.refresh_rating_extras_of_products(ARRAY( + SELECT x.product_id + FROM changed_new n JOIN changed_old o ON o.id = n.id, + LATERAL (VALUES (n.product_id), (o.product_id)) x(product_id) + WHERE (n.product_id, n.rating_star, n.rating_exact, n.criteria_ratings, + n.helpfulness_weight, n.reviewer_type, n.origin) + IS DISTINCT FROM + (o.product_id, o.rating_star, o.rating_exact, o.criteria_ratings, + o.helpfulness_weight, o.reviewer_type, o.origin))); ELSE - PERFORM rate.refresh_rating_extras_of_masters(ARRAY( - SELECT p.cluster_id FROM changed_old r JOIN rate.products p ON p.id = r.product_id)); + PERFORM rate.refresh_rating_extras_of_products(ARRAY(SELECT product_id FROM changed_old)); END IF; RETURN NULL; END $$; @@ -334,20 +377,19 @@ SET search_path = '' AS $$ BEGIN IF TG_OP = 'INSERT' THEN - PERFORM rate.refresh_rating_extras_of_masters(ARRAY( - SELECT p.cluster_id FROM changed_new f - JOIN rate.reviews r ON r.id = f.review_id JOIN rate.products p ON p.id = r.product_id)); + PERFORM rate.refresh_rating_extras_of_products(ARRAY( + SELECT r.product_id FROM changed_new f JOIN rate.reviews r ON r.id = f.review_id)); ELSIF TG_OP = 'UPDATE' THEN - PERFORM rate.refresh_rating_extras_of_masters(ARRAY( - SELECT p.cluster_id FROM (SELECT review_id FROM changed_new - UNION SELECT review_id FROM changed_old) f - JOIN rate.reviews r ON r.id = f.review_id JOIN rate.products p ON p.id = r.product_id)); + PERFORM rate.refresh_rating_extras_of_products(ARRAY( + SELECT r.product_id + FROM changed_new n JOIN changed_old o ON o.review_id = n.review_id + JOIN rate.reviews r ON r.id = n.review_id + WHERE n.include_in_rating IS DISTINCT FROM o.include_in_rating)); ELSE -- A flag deleted with its review (ON DELETE CASCADE) finds no review here; the reviews -- trigger refreshes that master. - PERFORM rate.refresh_rating_extras_of_masters(ARRAY( - SELECT p.cluster_id FROM changed_old f - JOIN rate.reviews r ON r.id = f.review_id JOIN rate.products p ON p.id = r.product_id)); + PERFORM rate.refresh_rating_extras_of_products(ARRAY( + SELECT r.product_id FROM changed_old f JOIN rate.reviews r ON r.id = f.review_id)); END IF; RETURN NULL; END $$; @@ -363,7 +405,7 @@ BEGIN RETURN NULL; END $$; -REVOKE EXECUTE ON FUNCTION rate.refresh_rating_extras_of_masters(uuid[]) FROM PUBLIC, anon, authenticated; +REVOKE EXECUTE ON FUNCTION rate.refresh_rating_extras_of_products(uuid[]) FROM PUBLIC, anon, authenticated; REVOKE EXECUTE ON FUNCTION rate.reviews_refresh_rating_extras() FROM PUBLIC, anon, authenticated; REVOKE EXECUTE ON FUNCTION rate.review_flags_refresh_rating_extras() FROM PUBLIC, anon, authenticated; REVOKE EXECUTE ON FUNCTION rate.master_products_refresh_rating_extras() FROM PUBLIC, anon, authenticated; @@ -410,7 +452,15 @@ COMMIT; -- Re-runnable; it writes only rows whose values differ. A catalog of many thousand reviewed -- masters can run it in id ranges instead. +-- The CHECKs above were added NOT VALID (no scan under the ACCESS EXCLUSIVE lock); validating +-- them here scans each table under a lock that lets reads and writes go on. BEGIN; +ALTER TABLE rate.reviews VALIDATE CONSTRAINT reviews_rating_exact_range; +ALTER TABLE rate.reviews VALIDATE CONSTRAINT reviews_verdict_length; +ALTER TABLE rate.reviews VALIDATE CONSTRAINT reviews_criteria_version_positive; +ALTER TABLE rate.reviews VALIDATE CONSTRAINT reviews_value_score_range; +ALTER TABLE rate.master_products VALIDATE CONSTRAINT master_products_rating_score_range; +ALTER TABLE rate.master_products VALIDATE CONSTRAINT master_products_rating_weight_sum_nonneg; SELECT rate.refresh_master_rating_extras(m.id) FROM rate.master_products m WHERE m.authentic_review_count > 0 OR m.user_rating_weighted > 0 diff --git a/scripts/test-criteria-ratings-proposal.mjs b/scripts/test-criteria-ratings-proposal.mjs index ba22d1608..bf672fad8 100644 --- a/scripts/test-criteria-ratings-proposal.mjs +++ b/scripts/test-criteria-ratings-proposal.mjs @@ -284,6 +284,41 @@ try { assert.equal(row(x128), pooledAfterVote, '128GB after the vote'); assert.equal(row(x256), pooledAfterVote, '256GB after the vote'); checkAll('after a vote on the Item path'); + // A review on an Item listing with no variant (cluster_id NULL) refreshes the Item too: + // weights 5 + 1 = 6, stars 19.3 + 4: (12.6 + 23.3) / 9 = 3.99. + const noVariant = 'b0000000-0000-4000-8000-0000000000a0'; + const noVariantReview = 'd0000000-0000-4000-8000-0000000000a0'; + sql(` + INSERT INTO rate.products (id, merchant_id, platform, platform_item_id, title, price_current, url, + cluster_id, item_id) + SELECT '${noVariant}', '${MERCHANT}', 'shopee', 'xnv', 'Acme Phone X', 11000000, + 'https://shopee.vn/xnv', NULL, item_id + FROM rate.master_products WHERE id = '${x128}'; + INSERT INTO rate.reviews (id, product_id, rating_star, comment, reviewer_type) + VALUES ('${noVariantReview}', '${noVariant}', 4, 'Máy ổn.', 'user'); + INSERT INTO rate.review_flags (review_id, verdict, include_in_rating, detector, detector_version, input_hash) + VALUES ('${noVariantReview}', 'authentic', true, 'replay', 'v1', 'hnv'); + UPDATE rate.reviews SET helpfulness_weight = 1 WHERE id = '${noVariantReview}'; + `); + assert.equal( + sql( + `SELECT cluster_id IS NULL FROM rate.products WHERE id = '${noVariant}'` + ), + 't', + 'the listing has no variant' + ); + const withNoVariant = + '3.99|6.00|{"display": {"score": 3.80, "writers": 4, "weight_sum": 4.00}}'; + assert.equal(row(x128), withNoVariant, '128GB counts the no-variant review'); + assert.equal(row(x256), withNoVariant, '256GB counts the no-variant review'); + // A re-crawl that rewrites the review unchanged refreshes nothing; V6 stays 0 either way. + sql( + `UPDATE rate.reviews SET comment = comment WHERE id = '${noVariantReview}'` + ); + checkAll('with a no-variant review'); + sql(`DELETE FROM rate.reviews WHERE id = '${noVariantReview}'; + DELETE FROM rate.products WHERE id = '${noVariant}';`); + assert.equal(row(x128), pooledAfterVote, '128GB after the review is deleted'); // 3. The M1+M2 rollback under the proposal: the triggers stay and the 128GB, whose review // changes, goes back to its own rating. The 256GB keeps the pooled values until something From 11194fc9ce170c383b016c2aa3c742646b532945 Mon Sep 17 00:00:00 2001 From: JOY <5027251+JOY@users.noreply.github.com> Date: Sun, 1 Nov 2026 02:04:20 +0700 Subject: [PATCH 4/5] chore(git): track the criteria ratings proposal sql files The Secret scan job refuses tracked files that match .gitignore. The three P2 proposal files get the same named exceptions as the earlier DOS-Me proposals. Refs #1543 Co-Authored-By: Claude Opus 5.5 --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index 48f5c2c3e..eb9fd6efb 100644 --- a/.gitignore +++ b/.gitignore @@ -149,6 +149,9 @@ playwright-report-item-model/ !Docs/Proposals/2026-10-29-dos-me-m3-m4.sql !Docs/Proposals/2026-10-29-dos-me-m3-m4-rollback.sql !Docs/Proposals/2026-10-29-dos-me-m3-m4-verify.sql +!Docs/Proposals/2026-10-31-dos-me-criteria-ratings.sql +!Docs/Proposals/2026-10-31-dos-me-criteria-ratings-rollback.sql +!Docs/Proposals/2026-10-31-dos-me-criteria-ratings-verify.sql * - Copy* * - Bản sao* *.dump From 21943656869d8330bf70d420dcce584819cccb7d Mon Sep 17 00:00:00 2001 From: JOY <5027251+JOY@users.noreply.github.com> Date: Sun, 1 Nov 2026 02:09:59 +0700 Subject: [PATCH 5/5] fix(proposals): store rating_score at three decimals so pages round once The web rounded the stored 2-decimal score again for display, so 4.249 showed 4.3. rating_score is numeric(4,3) and the criterion scores keep 3 decimals too; pages round once, to 1 decimal. Replay expectations follow. Refs #1543 Co-Authored-By: Claude Opus 5.5 --- .../2026-10-31-dos-me-criteria-ratings.md | 2 +- .../2026-10-31-dos-me-criteria-ratings.sql | 7 +++--- scripts/test-criteria-ratings-proposal.mjs | 24 +++++++++---------- 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/Docs/Proposals/2026-10-31-dos-me-criteria-ratings.md b/Docs/Proposals/2026-10-31-dos-me-criteria-ratings.md index f6acbbf3c..b8c8bbe12 100644 --- a/Docs/Proposals/2026-10-31-dos-me-criteria-ratings.md +++ b/Docs/Proposals/2026-10-31-dos-me-criteria-ratings.md @@ -33,7 +33,7 @@ Design: [`2026-10-31-criteria-stars-pros-cons.html`](./2026-10-31-criteria-stars - Public read; only the service role writes. - **`rate.product_points`:** the aggregated lines of a master, with support, writer count, an expert flag, and first and last seen dates. Public read; the service role and the worker (P5) write. - **`rate.master_products`:** - - `rating_score`: the one rating of D3. It is a Bayesian average with k = 3 and mu = 4.2 (the same formula as the web's `computeBayesianRatingSummary`). Each review weight is capped at 3, and `rating_exact` is used when present. + - `rating_score`: the one rating of D3. It is a Bayesian average with k = 3 and mu = 4.2 (the same formula as the web's `computeBayesianRatingSummary`). Each review weight is capped at 3, and `rating_exact` is used when present. It is stored as `numeric(4,3)`, and criterion scores at 3 decimals too, so a page rounds once, to 1 decimal (4.249 shows 4.2). - `rating_weight_sum`. - `criteria_scores`: `{key: {score, weight_sum, writers}}`. A criterion appears only when all of these hold (D9): - 3 or more reviews rated it; diff --git a/Docs/Proposals/2026-10-31-dos-me-criteria-ratings.sql b/Docs/Proposals/2026-10-31-dos-me-criteria-ratings.sql index b431fdaab..c783187c9 100644 --- a/Docs/Proposals/2026-10-31-dos-me-criteria-ratings.sql +++ b/Docs/Proposals/2026-10-31-dos-me-criteria-ratings.sql @@ -128,7 +128,7 @@ GRANT SELECT ON rate.review_points TO rate_worker; -- 5. rate.master_products ---------------------------------------------------------------------- ALTER TABLE rate.master_products - ADD COLUMN IF NOT EXISTS rating_score numeric(3, 2), + ADD COLUMN IF NOT EXISTS rating_score numeric(4, 3), ADD COLUMN IF NOT EXISTS rating_weight_sum numeric(10, 2) NOT NULL DEFAULT 0, ADD COLUMN IF NOT EXISTS criteria_scores jsonb NOT NULL DEFAULT '{}'::jsonb; @@ -157,6 +157,7 @@ END $$; -- at 3 and the star = rating_exact when a criteria review set it, else -- rating_star (apps/web/src/lib/rate-domain-core.ts computeBayesianRatingSummary). -- NULL with no weight, so a page shows "no rating yet" instead of the prior. +-- Stored at 3 decimals (scores too), so a page rounds once, to 1 decimal. -- criteria = the same estimate per criterion of criteria_ratings, over the reviews that -- rated it with a number from 1 to 5 under a key shaped like the shared-data -- keys (^[a-z_]{2,32}$); kept only with 3 or more reviews and a weight sum of @@ -212,7 +213,7 @@ BEGIN ), overall AS ( SELECT round(COALESCE(sum(w), 0), 2) AS weight_sum, - CASE WHEN sum(w) > 0 THEN round((3 * 4.2 + sum(w * star)) / (3 + sum(w)), 2) END AS score + CASE WHEN sum(w) > 0 THEN round((3 * 4.2 + sum(w * star)) / (3 + sum(w)), 3) END AS score FROM eligible WHERE star BETWEEN 1 AND 5 ), @@ -220,7 +221,7 @@ BEGIN SELECT c.key, count(*) AS writers, round(sum(e.w), 2) AS weight_sum, - round((3 * 4.2 + sum(e.w * (c.value #>> '{}')::numeric)) / (3 + sum(e.w)), 2) AS score + round((3 * 4.2 + sum(e.w * (c.value #>> '{}')::numeric)) / (3 + sum(e.w)), 3) AS score FROM eligible e CROSS JOIN LATERAL jsonb_each( CASE WHEN jsonb_typeof(e.criteria_ratings) = 'object' THEN e.criteria_ratings diff --git a/scripts/test-criteria-ratings-proposal.mjs b/scripts/test-criteria-ratings-proposal.mjs index bf672fad8..6288df75a 100644 --- a/scripts/test-criteria-ratings-proposal.mjs +++ b/scripts/test-criteria-ratings-proposal.mjs @@ -113,12 +113,12 @@ try { `); // Per master (no Items yet). 128GB: weights 1 + 1 + 1 + 2 = 5 (review 4 is excluded), stars - // 5, 4.3 (exact), 3, 5: (3 * 4.2 + 22.3) / 8 = 4.36. display: 3 writers, weight 3: - // (12.6 + 12) / 6 = 4.10. value has 2 writers (hidden); a bad key and a string star are not - // criteria. 256GB: (12.6 + 2) / 4 = 3.65, one writer, no criteria. + // 5, 4.3 (exact), 3, 5: (3 * 4.2 + 22.3) / 8 = 4.3625, stored 4.363. display: 3 writers, weight 3: + // (12.6 + 12) / 6 = 4.100. value has 2 writers (hidden); a bad key and a string star are not + // criteria. 256GB: (12.6 + 2) / 4 = 3.650, one writer, no criteria. const per128 = - '4.36|5.00|{"display": {"score": 4.10, "writers": 3, "weight_sum": 3.00}}'; - const per256 = '3.65|1.00|{}'; + '4.363|5.00|{"display": {"score": 4.100, "writers": 3, "weight_sum": 3.00}}'; + const per256 = '3.650|1.00|{}'; const applyAndCheckPerMaster = (when) => { sql(proposal); @@ -144,7 +144,7 @@ try { ); assert.equal( row(x128), - '4.56|4.00|{}', + '4.557|4.00|{}', `review 3 out of the rating ${when}` ); sql( @@ -257,12 +257,12 @@ try { // 2. The proposal first, then M0 and M1+M2: the two sizes become one Item, and both // variants show the Item's pooled rating. Weights 5 + 1 = 6, stars 22.3 + 2: - // (12.6 + 24.3) / 9 = 4.10. display: 4 writers, weight 4: (12.6 + 14) / 7 = 3.80. + // (12.6 + 24.3) / 9 = 4.100. display: 4 writers, weight 4: (12.6 + 14) / 7 = 3.800. applyAndCheckPerMaster('before M1+M2'); sql(m0); sql(m1m2); const pooled = - '4.10|6.00|{"display": {"score": 3.80, "writers": 4, "weight_sum": 4.00}}'; + '4.100|6.00|{"display": {"score": 3.800, "writers": 4, "weight_sum": 4.00}}'; assert.equal( sql( `SELECT count(DISTINCT item_id) FROM rate.master_products WHERE id IN ('${x128}', '${x256}')` @@ -274,18 +274,18 @@ try { assert.equal(row(x256), pooled, '256GB shows the Item rating'); checkAll('after M1+M2 on top'); // A vote's weight change on the Item path (refresh_item_rating, not the master refresh): - // review 5 at weight 1: weights 5, stars 5 + 4.3 + 3 + 5 + 2 = 19.3, (12.6 + 19.3) / 8 = 3.99. + // review 5 at weight 1: weights 5, stars 5 + 4.3 + 3 + 5 + 2 = 19.3, (12.6 + 19.3) / 8 = 3.9875, stored 3.988. sql( `UPDATE rate.reviews SET helpfulness_weight = 1 WHERE id = '${review(5)}'` ); asRole('rate_worker', `SELECT rate.refresh_master_product_rating('${x256}')`); const pooledAfterVote = - '3.99|5.00|{"display": {"score": 3.80, "writers": 4, "weight_sum": 4.00}}'; + '3.988|5.00|{"display": {"score": 3.800, "writers": 4, "weight_sum": 4.00}}'; assert.equal(row(x128), pooledAfterVote, '128GB after the vote'); assert.equal(row(x256), pooledAfterVote, '256GB after the vote'); checkAll('after a vote on the Item path'); // A review on an Item listing with no variant (cluster_id NULL) refreshes the Item too: - // weights 5 + 1 = 6, stars 19.3 + 4: (12.6 + 23.3) / 9 = 3.99. + // weights 5 + 1 = 6, stars 19.3 + 4: (12.6 + 23.3) / 9 = 3.989. const noVariant = 'b0000000-0000-4000-8000-0000000000a0'; const noVariantReview = 'd0000000-0000-4000-8000-0000000000a0'; sql(` @@ -308,7 +308,7 @@ try { 'the listing has no variant' ); const withNoVariant = - '3.99|6.00|{"display": {"score": 3.80, "writers": 4, "weight_sum": 4.00}}'; + '3.989|6.00|{"display": {"score": 3.800, "writers": 4, "weight_sum": 4.00}}'; assert.equal(row(x128), withNoVariant, '128GB counts the no-variant review'); assert.equal(row(x256), withNoVariant, '256GB counts the no-variant review'); // A re-crawl that rewrites the review unchanged refreshes nothing; V6 stays 0 either way.