From 74234aaff29680659a42c2839daae236eb9af412 Mon Sep 17 00:00:00 2001 From: thephez Date: Thu, 29 Oct 2026 16:34:04 -0400 Subject: [PATCH] fix(dashrate): decode grouped-count rating keys for protocol v14 Protocol version 14 (dashpay/platform#5350) stores unsigned integer index values in value order, so a grouped `count` by rating now returns keys "01".."05" instead of the sign-flipped "81".."85". dashrate looked up the old keys, so every per-star count read 0, the derived average was null, and every resource showed "No reviews yet" even though reviews exist. Decode the returned keys with `ratingFromKeyHex` instead of re-encoding expected ones. Masking off the top bit handles both the PV14 and pre-PV14 forms. Apply the same fix to dashrate-lite.html. Unit tests now feed literal network-format keys rather than stubs built with the app's own encoder, which is why the break went unnoticed. A new read-only e2e spec checks on live testnet, for both the app and the lite page, that the per-star distribution sums to the total review count. It fails against the previous code. Closes #119 Co-Authored-By: Claude Opus 5.5 --- example-apps/dashrate/CLAUDE.md | 4 +- .../dashrate/public/dashrate-lite.html | 25 ++--- example-apps/dashrate/src/dash/queries.ts | 33 ++++--- .../dashrate/test/e2e/ratings.spec.ts | 96 +++++++++++++++++++ example-apps/dashrate/test/queries.test.ts | 55 +++++++++-- 5 files changed, 179 insertions(+), 34 deletions(-) create mode 100644 example-apps/dashrate/test/e2e/ratings.spec.ts diff --git a/example-apps/dashrate/CLAUDE.md b/example-apps/dashrate/CLAUDE.md index 69c85c1f..d34218eb 100644 --- a/example-apps/dashrate/CLAUDE.md +++ b/example-apps/dashrate/CLAUDE.md @@ -32,7 +32,7 @@ This app is the showcase for Platform 4.0's relational query features — provab - **[src/session/types.ts](src/session/types.ts)** — the `Session` shape (`{ sdk, keyManager, identityId }`) shared by App and the hooks. - **[src/lib/](src/lib/)** — pure utilities, no SDK references: [logger.ts](src/lib/logger.ts) (`Logger`/`LogLevel` types, `errorMessage`, `consoleLogger`), [format.ts](src/lib/format.ts) (`formatAverage`, `formatDate`, `shortId`), [ratings.ts](src/lib/ratings.ts) (`RATING_ROWS`, `emptySummary`/`emptyDistribution`, `ownerLabel`, `reviewCountLine`, text `stars`). - **[test/](test/)** — Vitest + Testing Library, flat directory, named after the subject. Three kinds: unit tests over `src/dash/` and `src/lib/` that stub the `DashSdk` shape (`*.test.ts`); component tests rendered with Testing Library (`*.test.tsx`); and hook tests via `renderHook`. Default Vitest env is `node`; component/hook tests opt into DOM with a `// @vitest-environment jsdom` pragma at the top of the file (the vitest `include` covers `**/*.test.{ts,tsx}`). Component/hook tests **mock the SDK loaders** (`vi.mock("../src/dash/sdkModule", …)` / `vi.mock("../src/dash/sdkCore", …)`) so the 8 MB bundle never imports — never let a test pull `@dashevo/evo-sdk` into the jsdom process. [tsconfig.app.json](tsconfig.app.json) includes `test`, so `tsc -b` (run by `build`) strict-typechecks the `.test.tsx` files — keep mock factories and stub props fully typed. `npm run test:coverage` runs the suite under v8 coverage. -- **[test/e2e/](test/e2e/)** — Playwright specs plus shared `fixtures.ts`, driven by [playwright.config.ts](playwright.config.ts), which auto-starts `npx vite` on port 5182. Two projects (`chromium-desktop` / `chromium-mobile`) so every spec exercises both layouts. Runs against real testnet — no SDK mocks. The specs are read-only shell smoke tests ([smoke](test/e2e/smoke.spec.ts) — navigation, browse a resource, How-it-works; [settings](test/e2e/settings.spec.ts) — the sign-in form renders/gates without signing in); they assert rendering, not live rating data. Run locally via `npm run test:e2e`. [tsconfig.app.json](tsconfig.app.json)'s `exclude: ["test/e2e"]` keeps these out of the app `tsc -b` (Playwright typechecks them itself). +- **[test/e2e/](test/e2e/)** — Playwright specs plus shared `fixtures.ts`, driven by [playwright.config.ts](playwright.config.ts), which auto-starts `npx vite` on port 5182. Two projects (`chromium-desktop` / `chromium-mobile`) so every spec exercises both layouts. Runs against real testnet — no SDK mocks. The specs are read-only: shell smoke tests ([smoke](test/e2e/smoke.spec.ts) — navigation, browse a resource, How-it-works; [settings](test/e2e/settings.spec.ts) — the sign-in form renders/gates without signing in; [lite](test/e2e/lite.spec.ts) — the single-file companion) assert rendering, not live rating data. The exception is [ratings](test/e2e/ratings.spec.ts), which checks a data-independent invariant on live data in both the app and the lite page: the grouped per-star distribution must sum to the ungrouped total count. It's the only test that would catch a network-side change to grouped-count key encoding. Run locally via `npm run test:e2e`. [tsconfig.app.json](tsconfig.app.json)'s `exclude: ["test/e2e"]` keeps these out of the app `tsc -b` (Playwright typechecks them itself). ## Review contract @@ -70,7 +70,7 @@ Same as the sibling apps: the `@dashevo/evo-sdk` browser bundle is ~8 MB and mus ## Gotchas -- **Grouped-count map keys are raw index-key bytes, NOT the value.** `count` with `groupBy: ["rating"]` returns a `Map` keyed by the hex of the property's order-preserving index-key encoding, not the integer. For a small positive integer that's the sign-flipped single byte `0x80 | value` → rating 5 is key `"85"`, rating 1 is `"81"` (verified against the live contract — it is _not_ an 8-byte big-endian form). `ratingKeyHex(r) = (0x80 | r).toString(16)` in [queries.ts](src/dash/queries.ts) re-encodes each known rating to look it up. The SDK exposes no decoder, so the client encodes the values it's looking for rather than decoding what comes back. +- **Grouped-count map keys are raw index-key bytes, NOT the value.** `count` with `groupBy: ["rating"]` returns a `Map` keyed by the hex of the property's order-preserving index-key encoding, not the integer. `rating` is a single unsigned byte, and its encoding changed with protocol version 14 ([dashpay/platform#5350](https://github.com/dashpay/platform/pull/5350), which also rekeys existing data): PV14+ returns the plain byte (rating 5 → `"05"`), ≤ PV13 returned the sign-flipped byte (rating 5 → `"85"`). `ratingFromKeyHex` in [queries.ts](src/dash/queries.ts) decodes either form by masking off the top bit; `getRatingDistribution` decodes the returned keys rather than looking up pre-encoded ones. The SDK exposes no decoder. Unit tests feed literal network-format keys — never build stub keys with the app's own encoder, or an encoding change goes unnoticed (that's how the PV14 break slipped through). - **`rangeCountable` is a separate flag from `countable`, required for range/grouped counts.** A range count (the `between` on `rating` that drives the grouped distinct walk) needs `rangeCountable: true` on the index, with the range field as the **last** index property. A `countable`-only index fails at query time with `range count requires a range_countable: true index whose last property matches the range field`. - **Don't mix `summable` and a deeper count-only index on a shared prefix** — it registers fine but breaks every document insert (`NotCountedOrSummed-wrapping is only supported for the six sum-bearing tree variants`). `resourceRatingAggregate` is intentionally count-only (no `summable`) so its `resourceId` value tree isn't count+sum and can host `resourceRatingDistribution`'s count-only `rating` continuation. The average is derived from the distribution instead of a `sum`/`average` query. Full analysis: [dashpay/platform#3960](https://github.com/dashpay/platform/issues/3960). - **`between` value is a 2-element array, inclusive.** `["rating","between",[1,5]]` matches `1 <= rating <= 5`. The drive expects exactly two bounds. diff --git a/example-apps/dashrate/public/dashrate-lite.html b/example-apps/dashrate/public/dashrate-lite.html index 48bfee0b..f7ab3f19 100644 --- a/example-apps/dashrate/public/dashrate-lite.html +++ b/example-apps/dashrate/public/dashrate-lite.html @@ -202,8 +202,13 @@

Recent reviews

return sdk; } - function ratingKeyHex(rating) { - return (0x80 | rating).toString(16); + // Grouped-count keys are the hex of the raw index-key byte: "05" for + // rating 5 on protocol v14+, "85" (sign bit flipped) before it. + // Masking off the top bit decodes both. + function ratingFromKeyHex(key) { + if (!/^[0-9a-f]{2}$/i.test(key)) return null; + const rating = parseInt(key, 16) & 0x7f; + return RATING_VALUES.includes(rating) ? rating : null; } function firstMapValue(counts) { @@ -212,11 +217,6 @@

Recent reviews

return Object.values(counts)[0]; } - function countValue(counts, key) { - const value = counts instanceof Map ? counts.get(key) : counts?.[key]; - return toBigInt(value, 0n); - } - function toBigInt(value, fallback = 0n) { if (typeof value === 'bigint') return value; if (typeof value === 'number' && Number.isFinite(value)) return BigInt(value); @@ -260,10 +260,13 @@

Recent reviews

orderBy: [['rating', 'asc']], groupBy: ['rating'], }); - return Object.fromEntries(RATING_VALUES.map((rating) => [ - rating, - countValue(counts, ratingKeyHex(rating)), - ])); + const distribution = Object.fromEntries(RATING_VALUES.map((rating) => [rating, 0n])); + const entries = counts instanceof Map ? counts.entries() : Object.entries(counts ?? {}); + for (const [key, value] of entries) { + const rating = ratingFromKeyHex(key); + if (rating !== null) distribution[rating] += toBigInt(value, 0n); + } + return distribution; } async function listResourceReviews(sdk, resourceId, ratingFilter) { diff --git a/example-apps/dashrate/src/dash/queries.ts b/example-apps/dashrate/src/dash/queries.ts index 2618452b..b2d1907e 100644 --- a/example-apps/dashrate/src/dash/queries.ts +++ b/example-apps/dashrate/src/dash/queries.ts @@ -49,18 +49,21 @@ export function firstMapValue(map: Map): T | undefined { } /** - * Index-key for an integer rating in a grouped `count` result map. + * Decode a grouped `count` result-map key back to its rating, or null if + * the key isn't a valid rating. * - * Platform encodes an integer index key in its order-preserving form: the - * sign bit is flipped so negatives sort before positives. For a small - * positive rating that's a single byte `0x80 | rating`, hex-encoded — e.g. - * rating 5 → `0x85` → "85". (Verified against the live contract: a grouped - * count over ratings returns keys "81".."85", NOT 8-byte big-endian.) The - * client builds the expected keys by encoding each known rating rather than - * decoding the returned keys. + * Keys are the hex of the raw index-key bytes, not the value. `rating` + * (integer 1–5) is stored as a single unsigned byte, and its encoding + * depends on the network's protocol version: + * - PV14+: plain value byte — rating 5 → `0x05` → "05" + * - ≤ PV13: sign bit flipped — rating 5 → `0x85` → "85" + * (dashpay/platform#5350; the PV14 upgrade rekeys existing data.) Masking + * off the top bit decodes both forms — a 1–5 rating never sets it. */ -export function ratingKeyHex(rating: number): string { - return (0x80 | rating).toString(16); +export function ratingFromKeyHex(key: string): number | null { + if (!/^[0-9a-f]{2}$/i.test(key)) return null; + const rating = parseInt(key, 16) & 0x7f; + return (RATING_VALUES as readonly number[]).includes(rating) ? rating : null; } function toTimestamp( @@ -218,8 +221,14 @@ export async function getRatingDistribution({ groupBy: ["rating"], }); const distribution: RatingDistribution = {}; - for (const rating of RATING_VALUES) { - distribution[rating] = counts.get(ratingKeyHex(rating)) ?? 0n; + for (const rating of RATING_VALUES) distribution[rating] = 0n; + for (const [key, count] of counts) { + const rating = ratingFromKeyHex(key); + if (rating === null) { + log(`Unexpected rating key "${key}" for ${resourceId}`, "error"); + continue; + } + distribution[rating] += count; } return distribution; } catch (err) { diff --git a/example-apps/dashrate/test/e2e/ratings.spec.ts b/example-apps/dashrate/test/e2e/ratings.spec.ts new file mode 100644 index 00000000..5dddf1b4 --- /dev/null +++ b/example-apps/dashrate/test/e2e/ratings.spec.ts @@ -0,0 +1,96 @@ +import { test, expect, type Page } from "./fixtures"; +import { test as liteTest } from "@playwright/test"; + +// Live rating-consistency checks against testnet. Unlike the smoke specs, +// these DO read live rating data, but only assert an invariant that holds +// for any data: the per-star distribution (grouped `count` by rating) must +// sum to the total review count (ungrouped `count`). Both come from separate +// queries, so a change in how Platform keys grouped-count results (e.g. the +// PV14 index-key encoding change, dashpay/platform#5350) shows up here as a +// non-zero total with an all-zero distribution. +// +// The default contract's reviews can't be deleted (`canBeDeleted: false`), +// so at least one resource always has reviews to check. + +const LOAD_TIMEOUT = 90_000; + +function parseCount(text: string): number { + const match = /(\d+)\s+reviews?/.exec(text); + return match ? Number(match[1]) : 0; +} + +async function sumCounts(page: Page, selector: string): Promise { + const texts = await page.locator(selector).allTextContents(); + return texts.reduce((sum, text) => sum + (parseInt(text, 10) || 0), 0); +} + +test.describe("rating consistency (app)", () => { + test("resources with reviews show a rating, and the distribution sums to the total", async ({ + page, + }) => { + const sidebar = page.locator('aside[aria-label="Tutorial resources"]'); + const rated = sidebar.locator("small.rating-present"); + await expect(rated.first()).toBeVisible({ timeout: LOAD_TIMEOUT }); + + // A card counted as having reviews (total > 0) must not fall back to + // "No reviews yet", which it does when the distribution is all zero. + for (const card of await rated.all()) { + await expect(card).not.toContainText("No reviews yet"); + } + + await sidebar + .locator("button.resource-card") + .filter({ has: page.locator("small.rating-present") }) + .first() + .click(); + + const stats = page.locator('[aria-label="Aggregate rating stats"]'); + await expect(stats.locator(".detail-rating-score")).not.toHaveText("—", { + timeout: LOAD_TIMEOUT, + }); + const total = parseCount( + (await stats.locator(".detail-rating-count").textContent()) ?? "", + ); + expect(total).toBeGreaterThan(0); + await expect + .poll(() => sumCounts(page, ".rating-histogram .histogram-count"), { + timeout: LOAD_TIMEOUT, + }) + .toBe(total); + }); +}); + +liteTest.describe("rating consistency (dashrate-lite)", () => { + liteTest( + "the distribution sums to the total for a resource with reviews", + async ({ page }) => { + liteTest.setTimeout(240_000); + await page.goto("/dashrate-lite.html"); + await expect(page.locator("#status")).toHaveText(/Connected to testnet/, { + timeout: LOAD_TIMEOUT, + }); + + const grid = page.locator("#aggregate-result .summary-grid"); + const navButtons = page.locator( + 'nav[aria-label="Tutorial resources"] button', + ); + let checked = false; + for (const button of await navButtons.all()) { + await button.click(); + await expect(grid).toBeVisible({ timeout: LOAD_TIMEOUT }); + const total = parseCount( + (await grid.locator(".score .label").textContent()) ?? "", + ); + if (total === 0) continue; + + await expect(grid.locator(".score .value")).not.toHaveText("-"); + expect( + await sumCounts(page, "#aggregate-result .dist-row .muted"), + ).toBe(total); + checked = true; + break; + } + expect(checked, "expected at least one resource with reviews").toBe(true); + }, + ); +}); diff --git a/example-apps/dashrate/test/queries.test.ts b/example-apps/dashrate/test/queries.test.ts index 7a8b6f7f..bb329af8 100644 --- a/example-apps/dashrate/test/queries.test.ts +++ b/example-apps/dashrate/test/queries.test.ts @@ -7,7 +7,7 @@ import { listResourceReviews, normalizeReviews, normalizeSingleReview, - ratingKeyHex, + ratingFromKeyHex, summaryFromDistribution, } from "../src/dash/queries"; import type { DashSdk } from "../src/dash/types"; @@ -140,20 +140,32 @@ describe("DashRate query normalization", () => { }); describe("DashRate rating distribution", () => { - it("encodes a rating as its order-preserving sign-flipped hex key", () => { - // 0x80 | rating — verified against the live contract's grouped count. - expect(ratingKeyHex(1)).toBe("81"); - expect(ratingKeyHex(5)).toBe("85"); + it("decodes PV14 plain-byte rating keys", () => { + // Literal keys as returned by testnet on protocol version 14. + expect(ratingFromKeyHex("01")).toBe(1); + expect(ratingFromKeyHex("05")).toBe(5); + }); + + it("decodes pre-PV14 sign-flipped rating keys", () => { + expect(ratingFromKeyHex("81")).toBe(1); + expect(ratingFromKeyHex("85")).toBe(5); + }); + + it("rejects keys that aren't a 1–5 rating", () => { + for (const key of ["", "00", "06", "80", "86", "0005", "zz"]) { + expect(ratingFromKeyHex(key)).toBeNull(); + } }); it("maps grouped count keys back to ratings, defaulting absent to 0", async () => { // Grouped `count` returns one entry per PRESENT rating, keyed by the - // order-preserving hex key. Ratings with no reviews (1, 2) are absent. + // hex index key. Ratings with no reviews (1, 2) are absent. Literal + // PV14 keys, not built with app code, so an encoding change shows up. const countMock = vi.fn().mockResolvedValue( new Map([ - [ratingKeyHex(3), 2n], - [ratingKeyHex(4), 5n], - [ratingKeyHex(5), 8n], + ["03", 2n], + ["04", 5n], + ["05", 8n], ]), ); const sdk = { documents: { count: countMock } } as unknown as DashSdk; @@ -180,6 +192,31 @@ describe("DashRate rating distribution", () => { groupBy: ["rating"], }); }); + + it("decodes legacy keys and logs (skips) unexpected ones", async () => { + const countMock = vi.fn().mockResolvedValue( + new Map([ + ["81", 1n], + ["85", 3n], + ["ff", 9n], + ]), + ); + const sdk = { documents: { count: countMock } } as unknown as DashSdk; + const log = vi.fn(); + + const distribution = await getRatingDistribution({ + sdk, + contractId: "contract-1", + resourceId: "tokens", + log, + }); + + expect(distribution).toEqual({ 1: 1n, 2: 0n, 3: 0n, 4: 0n, 5: 3n }); + expect(log).toHaveBeenCalledWith( + 'Unexpected rating key "ff" for tokens', + "error", + ); + }); }); describe("DashRate review list filtering", () => {