From 69e9f51aa274746d13a03f5029d8fbce035fc12b Mon Sep 17 00:00:00 2001 From: JOY <5027251+JOY@users.noreply.github.com> Date: Sun, 1 Nov 2026 12:31:10 +0700 Subject: [PATCH] fix(shared-data): product points second review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - An expert line lifts a point only while it is fresh. - Lines that name different aspects or numbers never join one cluster. - A short line inside a longer one joins it, whichever is older. - Duplicate line ids count once; writer ids are compared trimmed. - A future date counts as now; "hề" is a filler, so "không hề nóng" negates. Refs #1546 Co-Authored-By: Claude Opus 5.5 --- .../src/__tests__/shared-data.test.ts | 50 +++++++++++ .../shared-data/src/rules/product-points.ts | 83 ++++++++++++++++--- 2 files changed, 121 insertions(+), 12 deletions(-) diff --git a/packages/shared-data/src/__tests__/shared-data.test.ts b/packages/shared-data/src/__tests__/shared-data.test.ts index 4e1204bc..511ce1ce 100644 --- a/packages/shared-data/src/__tests__/shared-data.test.ts +++ b/packages/shared-data/src/__tests__/shared-data.test.ts @@ -1801,6 +1801,28 @@ function testProductPoints(): void { line('b', 'con', 'Màn hình không bị ám vàng'), ]); assert.equal(negated.clusters.length, 2, 'a negation splits one kind'); + assert.deepEqual(rules.pointTokens('máy không hề nóng'), [ + 'máy', + 'không_nóng', + ]); + // Lines that share a frame but state different facts stay apart: another + // aspect ("nóng" against "lag") or another number. + const claims = (a: string, b: string) => + run([line('a', 'con', a), line('b', 'con', b)]).clusters.length; + assert.equal(claims('máy nóng khi chơi game', 'máy lag khi chơi game'), 2); + assert.equal(claims('pin dùng được 1 ngày', 'pin dùng được 2 ngày'), 2); + // A short line inside a longer one joins it, whichever is older. + for (const ids of [ + ['x1', 'x2'], + ['x2', 'x1'], + ]) + assert.equal( + run([ + line('a', 'pro', 'pin trâu', { id: ids[0] }), + line('b', 'pro', 'pin trâu dùng cả ngày', { id: ids[1] }), + ]).clusters.length, + 1 + ); const alike = run([ line('a', 'pro', 'Pin trâu, dùng cả ngày'), line('b', 'pro', 'pin rất trâu dùng cả ngày'), @@ -1890,6 +1912,34 @@ function testProductPoints(): void { line('h', 'pro', 'pin trâu cả ngày', { weight: Number.NaN }), ]); assert.equal(malformed.clusters.length, 0); + // A line read twice counts once; writer ids are compared trimmed. + const twice = line('a', 'con', 'loa rè khi mở to'); + const dup = run([ + twice, + twice, + { ...twice, writerId: 'b' }, + line(' c', 'con', 'loa rè khi mở to'), + line('c ', 'con', 'loa rè khi mở to'), + ]).clusters[0]!; + assert.deepEqual(dup.lineIds.length, 3); + assert.equal(dup.reviewerCount, 2); + // An old expert line does not lift one fresh writer's point, and a future + // date counts as now. + const stale = run([ + line('x', 'con', 'pin tụt nhanh', { + isExpert: true, + writtenAt: monthsAgo(60), + }), + line('h', 'con', 'pin tụt rất nhanh'), + ]).clusters[0]!; + assert.equal(stale.hasExpert, false); + assert.equal(stale.visible, false); + const future = run([ + line('a', 'pro', 'loa to rõ', { + writtenAt: new Date(now.getTime() + 86_400_000 * 400).toISOString(), + }), + ]).clusters[0]!; + assert.equal(future.lastSeen.getTime(), now.getTime()); // Freshness: a weight halves every 12 months; 18 silent months hide a point. assert.ok( diff --git a/packages/shared-data/src/rules/product-points.ts b/packages/shared-data/src/rules/product-points.ts index e4fbfb1d..5cc26a62 100644 --- a/packages/shared-data/src/rules/product-points.ts +++ b/packages/shared-data/src/rules/product-points.ts @@ -119,6 +119,7 @@ const FILLER = new Set([ 'ạ', 'nhé', 'nha', + 'hề', ]); const NEGATED = 'không_'; // A negation joins the next word, so "pin không trâu" never matches @@ -343,6 +344,8 @@ export function decayedWeight( interface Line { source: PointLine; + /** The writer id, trimmed. */ + writer: string; criterionKey: string | null; tokens: string[]; form: string; @@ -368,22 +371,74 @@ function contradicts(a: readonly string[], b: readonly string[]): boolean { return negates(a, new Set(b)) || negates(b, new Set(a)); } +/** Single-word keywords: a word that names an aspect ("nóng", "lag"). */ +const ASPECT_WORDS: ReadonlySet = new Set( + Object.values(CRITERION_KEYWORDS) + .flat() + .filter((keyword) => !keyword.includes(SPACE)) +); + +/** + * Two lines that state different facts while sharing most words: each names + * an aspect the other does not ("máy nóng khi chơi game" against "máy lag + * khi chơi game"), or their numbers differ ("1 ngày" against "2 ngày"). + */ +function differentClaims(a: readonly string[], b: readonly string[]): boolean { + const left = new Set(a); + const right = new Set(b); + const only = ( + from: Set, + to: Set, + test: (word: string) => boolean + ): boolean => [...from].some((word) => test(word) && !to.has(word)); + const isAspect = (word: string): boolean => ASPECT_WORDS.has(word); + const isNumber = (word: string): boolean => /^\p{N}+$/u.test(word); + return ( + (only(left, right, isAspect) && only(right, left, isAspect)) || + only(left, right, isNumber) || + only(right, left, isNumber) + ); +} + +/** + * How well a line matches a cluster's first line: 0 when they contradict or + * state different facts; at least POINT_CLUSTER_OVERLAP when every word of + * the shorter one (two words or more) is in the longer one ("pin trâu" in + * "pin trâu dùng cả ngày"); otherwise their word overlap. + */ +function matchScore(a: readonly string[], b: readonly string[]): number { + if (contradicts(a, b) || differentClaims(a, b)) return 0; + const overlap = wordOverlap(a, b); + const [shorter, longer] = a.length <= b.length ? [a, b] : [b, a]; + const inside = new Set(longer); + const contained = + new Set(shorter).size >= 2 && shorter.every((word) => inside.has(word)); + return contained ? Math.max(overlap, POINT_CLUSTER_OVERLAP) : overlap; +} + /** * The lines that may count: a writer, a finite positive weight, a date, a * length rate.review_points allows, words in it, and not a dodge. */ function countedLines( categorySlug: string | null | undefined, - lines: readonly PointLine[] + lines: readonly PointLine[], + now: Date ): Line[] { const counted: Line[] = []; + const seen = new Set(); for (const source of lines) { + // A line read twice counts once. + if (seen.has(source.id)) continue; + seen.add(source.id); if (source.kind !== 'pro' && source.kind !== 'con') continue; if (!Number.isFinite(source.weight) || source.weight <= 0) continue; if (typeof source.writerId !== 'string' || !source.writerId.trim()) continue; - const writtenAt = asDate(source.writtenAt); - if (Number.isNaN(writtenAt.getTime())) continue; + const dated = asDate(source.writtenAt); + if (Number.isNaN(dated.getTime())) continue; + // A future date is now: it neither escapes decay nor keeps a point fresh. + const writtenAt = dated.getTime() > now.getTime() ? now : dated; const body = String(source.body ?? EMPTY).trim(); const length = [...body.normalize('NFKC')].length; if (length < REVIEW_POINT_MIN || length > REVIEW_POINT_MAX) continue; @@ -393,6 +448,7 @@ function countedLines( if (tokens.length === 0) continue; counted.push({ source, + writer: source.writerId.trim(), criterionKey: assignPointCriterion( categorySlug, source.criterionKey, @@ -429,9 +485,7 @@ function clusterLines(lines: readonly Line[]): Line[][] { leader.source.kind !== line.source.kind ) continue; - const overlap = contradicts(leader.tokens, line.tokens) - ? 0 - : wordOverlap(leader.tokens, line.tokens); + const overlap = matchScore(leader.tokens, line.tokens); if (overlap >= bestOverlap && (!best || overlap > bestOverlap)) { best = cluster; bestOverlap = overlap; @@ -448,10 +502,10 @@ function clusterLabel(lines: readonly Line[]): string { const forms = new Map; first: Line }>(); for (const line of lines) { const entry = forms.get(line.form); - if (entry) entry.writers.add(line.source.writerId); + if (entry) entry.writers.add(line.writer); else forms.set(line.form, { - writers: new Set([line.source.writerId]), + writers: new Set([line.writer]), first: line, }); } @@ -468,9 +522,14 @@ function summarise(lines: readonly Line[], now: Date): PointCluster { let hasExpert = false; for (const line of lines) { const weight = decayedWeight(line.source.weight, line.writtenAt, now); - const writer = line.source.writerId; - writers.set(writer, Math.max(writers.get(writer) ?? 0, weight)); - if (line.source.isExpert) hasExpert = true; + writers.set(line.writer, Math.max(writers.get(line.writer) ?? 0, weight)); + // An expert counts while their line is fresh: an old expert line does not + // lift a new single writer's point (design 5.3). + if ( + line.source.isExpert && + now.getTime() - line.writtenAt.getTime() <= POINT_SILENT_MS + ) + hasExpert = true; } let rawSupport = 0; for (const weight of writers.values()) rawSupport += weight; @@ -526,7 +585,7 @@ export function aggregateProductPoints( lines: readonly PointLine[], now: Date ): ProductPoints { - const counted = countedLines(categorySlug, lines); + const counted = countedLines(categorySlug, lines, now); const clusters = clusterLines(counted) .map((cluster) => summarise(cluster, now)) .sort(byRank);