diff --git a/delphi/docs/CLJ-PARITY-FIXES-JOURNAL.md b/delphi/docs/CLJ-PARITY-FIXES-JOURNAL.md index 49e6b3bd8..e25950760 100644 --- a/delphi/docs/CLJ-PARITY-FIXES-JOURNAL.md +++ b/delphi/docs/CLJ-PARITY-FIXES-JOURNAL.md @@ -1951,3 +1951,54 @@ regen + true D12 value-parity is now **follow-up work under #2571, blocked on ex the generator's vote-copy delta so cold-start inputs match; (3) THEN un-mirror `priority_metric`, regenerate cold-start blobs, and change D12's test from the trivial constant-49 check to a real varied-value / rank-parity assertion. + +### Session: D1b — fix `pca_project_cmnts` comment-extremity sign (2026-07-17) + +**Bug.** `pca_project_cmnts` (`polismath/pca_kmeans_rep/pca.py`) computed +`coefs = -scale * (1.0 + center)` — a literal, untranslated copy of Clojure's +synthetic vote value `-1` (`math/src/polismath/math/pca.clj:167-178`). In Clojure +that `-1` is correct because Clojure stays in raw-Postgres convention throughout, +where AGREE = -1 and `center` is a mean in that same convention. Delphi flips +votes to its own convention at the Postgres ingress (`postgres_vote_to_delphi`), +so the PCA is fit on AGREE = +1 data and `center` is a Delphi-convention mean. +Projecting the untranslated `-1` therefore **inverts comment extremity**: +`|correct| = scale·|1 − center|` vs `|actual| = scale·|1 + center|` — equal only +at `center == 0`. A near-unanimous-AGREE comment (`center → +1`) should have +extremity → 0 but the buggy code reported `2·scale` (maximally extreme); a +near-unanimous-DISAGREE comment (`center → −1`) should be maximal but reported ≈0. +The consensus↔extremity relationship was reversed. + +**Fix.** `coefs = scale * (AGREE - center)` (AGREE = +1, imported from +`utils.general`). Faithful Delphi-convention port of the Clojure synthetic-AGREE +projection. Docstring rewritten to explain the convention translation. + +**Why no test caught it.** (1) The old `test_pca_project_cmnts_formula` was +tautological — it re-derived the implementation's own `-scale*(1+center)`. (2) The +end-to-end golden/legacy comparisons compare priorities that BOTH sides +short-circuit to the constant 49 under the #2571 bug-mirror, and the fixtures +where extremity would matter are xfail-marked. A convention mismatch between the +PCA-fit stage and the comment-projection stage was structurally unobservable. + +**Tests (TDD, RED→GREEN).** Replaced the tautological formula test with one +deriving the expected value independently from the `AGREE` constant; added a +behavioral sign test (agree → extremity 0, disagree → max); added an integration +test on `_compute_comment_priorities` that spies on the extremity `E` reaching +`priority_metric` (works despite the #2571 mirror, since it inspects the argument, +not the return) and pins it to hand-derived values (0 and 2·√2). Also added a +provenance comment at `regression/utils.py` recording that the regression CSVs are +pre-flipped to Delphi convention by `server/src/report.ts` (~line 393, +`vote: String(-row.vote)`), so the regression path must NOT re-flip. + +**Output-inert today.** Because `priority_metric` still returns +`META_PRIORITY**2` (the #2571 mirror), extremity affects no DynamoDB output yet — +full suite **406 passed / 17 skipped / 47 xfailed / 0 failed**, and **no golden +snapshots moved**. The fix becomes live when the mirror is removed; it is +exactly the extremity/PCA-parity groundwork that the D12 un-mirror is blocked on. + +**Not D1.** Distinct from the `align_pca_signs()` eigenvector-orientation +stability fix (`jc/clj-parity-d1-pca-sign-flip-prevention`) — that is temporal +±sign ambiguity between ticks, unrelated to this projection-convention bug. + +**What's next:** with D1b closed, the remaining blockers on the D12 un-mirror are +the D1 sign-stability work and the participant-filtering / vote-copy reconciliation +noted above. diff --git a/delphi/docs/PLAN_DISCREPANCY_FIXES.md b/delphi/docs/PLAN_DISCREPANCY_FIXES.md index 449661cbf..8bb188bd9 100644 --- a/delphi/docs/PLAN_DISCREPANCY_FIXES.md +++ b/delphi/docs/PLAN_DISCREPANCY_FIXES.md @@ -527,7 +527,7 @@ See `delphi/docs/INVESTIGATION_K_DIVERGENCE.md` for the full investigation. | ID | Discrepancy | Plan PR | GitHub PR | Status | |----|-------------|---------|-----------|--------| | D1 | PCA sign flips | PR 13 | — (WIP) | VM draft — **NEEDS REWORK** (no replay tests) | -| D1b | Projection input | PR 13 | — (WIP) | VM draft — documented, low severity, no code change | +| D1b | Projection input | PR 13 | — (spr-stack) | **CODE FIX DONE ✓ (2026-07-17)** — `pca_project_cmnts` projected the untranslated Clojure literal `-1` (`-scale*(1+center)`) instead of the Delphi `AGREE` constant, INVERTING comment extremity (near-unanimous-agree → maximally extreme). Fixed to `scale*(AGREE-center)`. 3 tests (formula derived from AGREE, agree/disagree sign, extremity→priority_metric spy). Output-inert today (masked by #2571 `priority_metric` short-circuit) → no golden movement. Unblocks the D12 un-mirror. **Distinct from D1** (align_pca_signs / temporal stability). | | D2 | In-conv threshold | **PR 1** | **#2513** | **DONE** ✓ | | D2b | Base-cluster sort order | **PR 1** | **#2513** | **DONE** ✓ | | D2c | Vote count source (raw vs filtered matrix) | **PR 1** | **#2513** | **DONE** ✓ | diff --git a/delphi/polismath/pca_kmeans_rep/pca.py b/delphi/polismath/pca_kmeans_rep/pca.py index 6ba273ad0..1b217bfca 100644 --- a/delphi/polismath/pca_kmeans_rep/pca.py +++ b/delphi/polismath/pca_kmeans_rep/pca.py @@ -11,6 +11,8 @@ import pandas as pd from typing import Dict, List, Optional, Sequence, Tuple, Union, Any +from polismath.utils.general import AGREE + logger = logging.getLogger(__name__) @@ -396,19 +398,31 @@ def pca_project_cmnts(center: np.ndarray, comps: np.ndarray) -> np.ndarray: Clojure (`pca-project-cmnts`, pca.clj:167-178) calls `sparsity-aware-project-ptpts` on a synthetic vote matrix where row `i` - has value `-1` at column `i` and `nil` everywhere else. + has a single AGREE vote at column `i` and `nil` everywhere else. For comment `i`, the sparsity-aware reduce (pca.clj:134-157) collapses to: n_votes = 1 (only column i is non-nil) - p1 = (-1 - center[i]) * pc1[i] - p2 = (-1 - center[i]) * pc2[i] + p1 = (agree_vote - center[i]) * pc1[i] + p2 = (agree_vote - center[i]) * pc2[i] scale = sqrt(n_cmnts / max(1, 1)) = sqrt(n_cmnts) Final row: - proj[i] = sqrt(n_cmnts) * (-1 - center[i]) * [pc1[i], pc2[i]] - = -sqrt(n_cmnts) * (1 + center[i]) * [pc1[i], pc2[i]] + proj[i] = sqrt(n_cmnts) * (agree_vote - center[i]) * [pc1[i], pc2[i]] + + **Convention note (D1b):** Clojure uses the literal vote value `-1` here + because Clojure stays in raw-Postgres convention throughout, where + AGREE = -1 (and its `center` is the mean in that same convention). Delphi + flips votes to its own convention at the Postgres ingress boundary + (`postgres_vote_to_delphi`), so the PCA is fit on AGREE = +1 data and + `center` is a mean in Delphi convention. The faithful port therefore + projects the Delphi `AGREE` constant (+1), NOT the untranslated literal -1. + + Using -1 here would invert comment extremity: `|AGREE - center|` correctly + sends a near-unanimous-AGREE comment (center → +1) to extremity ~0 and a + near-unanimous-DISAGREE comment (center → -1) to maximal extremity; + `-(1 + center)` reverses both. The two agree only at center == 0. Args: - center: PCA center (column means), shape (n_cmnts,). + center: PCA center (column means, Delphi convention), shape (n_cmnts,). comps: PCA components, shape (n_components, n_cmnts). Typically n_components == 2. @@ -420,7 +434,7 @@ def pca_project_cmnts(center: np.ndarray, comps: np.ndarray) -> np.ndarray: if n_cmnts == 0: return np.zeros((0, comps.shape[0] if comps.ndim == 2 else 0)) scale = np.sqrt(n_cmnts) - coefs = -scale * (1.0 + center) # shape (n_cmnts,) + coefs = scale * (AGREE - center) # shape (n_cmnts,); AGREE = +1 (Delphi) return coefs[:, None] * comps.T # shape (n_cmnts, n_components) diff --git a/delphi/polismath/regression/utils.py b/delphi/polismath/regression/utils.py index 1e2e67120..f65273ab9 100644 --- a/delphi/polismath/regression/utils.py +++ b/delphi/polismath/regression/utils.py @@ -264,6 +264,15 @@ def prepare_votes_data(dataset_name: str) -> Tuple[Dict, Dict[str, Any]]: # Convert votes DataFrame to the format expected by update_votes # Expected format: {'pid': voter_id, 'tid': comment_id, 'vote': vote_value, 'created': timestamp} + # + # Vote convention: these CSVs are exported by the TS server + # (server/src/report.ts ~393 — `vote: String(-row.vote) // flip -1 to 1`), + # which ALREADY flips raw Postgres (AGREE=-1) into Delphi convention + # (AGREE=+1). `update_votes` expects Delphi convention and does NOT re-flip + # (the `postgres_vote_to_delphi` ingress flip lives on the live-Postgres path + # in run_math_pipeline.py / database/postgres.py, not here). So `row['vote']` + # is passed through as-is — do NOT add a flip here, or PCA center/extremity + # (which assume Delphi convention) would be inverted. votes_list = [] for _, row in votes_df.iterrows(): votes_list.append({ diff --git a/delphi/tests/test_discrepancy_fixes.py b/delphi/tests/test_discrepancy_fixes.py index 62bf63da4..8039acf4a 100644 --- a/delphi/tests/test_discrepancy_fixes.py +++ b/delphi/tests/test_discrepancy_fixes.py @@ -2069,6 +2069,60 @@ def test_desync_returns_empty_and_logs(self, caplog): for r in caplog.records), \ "expected an ERROR log naming the priorities/extremity desync" + def test_extremity_sign_reaches_priority_metric(self, monkeypatch): + """End-to-end sign check through `_compute_comment_priorities`. + + `priority_metric` currently short-circuits to `META_PRIORITY**2` (the + #2571 Clojure-bug mirror), so we can't assert on its RETURN value. But + the extremity `E` it is CALLED with is exactly what the pca sign bug + corrupts. We spy on that argument (independent of the mirror) and pin + it to a hand-derived value. + + Setup: two comments, one near-unanimous AGREE (center +1), one + near-unanimous DISAGREE (center -1), with pc1 = 1 / pc2 = 0 so + extremity == |coef|. Correct convention translation ⇒ agree extremity 0, + disagree extremity 2·sqrt(2). The pre-fix untranslated `-1` inverts them. + + `group_clusters` is left empty on purpose: A/P/S collapse to 0 for every + tid, so the only quantity varying between the two calls is `E` — no + confound from vote aggregation. + """ + import polismath.conversation.conversation as convmod + + conv = Conversation(conversation_id='ztest-extremity-sign') + conv.rating_mat = pd.DataFrame( + [[1.0, -1.0], [1.0, -1.0], [1.0, -1.0]], # 3 ptpts; col 10 agree, col 11 disagree + index=[0, 1, 2], columns=[10, 11], + ) + conv.raw_rating_mat = conv.rating_mat.copy() + conv.pca = { + 'center': np.array([1.0, -1.0]), + 'comps': np.array([[1.0, 1.0], [0.0, 0.0]]), + } + conv.group_clusters = [] + conv.meta_tids = set() + + captured_E = [] + real_priority_metric = convmod.priority_metric + + def spy(is_meta, A, P, S, E): + captured_E.append(E) + return real_priority_metric(is_meta, A, P, S, E) + + monkeypatch.setattr(convmod, 'priority_metric', spy) + conv._compute_comment_priorities() + + # Call order follows rating_mat.columns == [10 (agree), 11 (disagree)]. + assert len(captured_E) == 2, f"expected one priority_metric call per tid, got {captured_E}" + e_agree, e_disagree = captured_E + scale = np.sqrt(2) + assert e_agree == pytest.approx(0.0, abs=1e-9), \ + "unanimous-agree comment must reach priority_metric with extremity ~0" + assert e_disagree == pytest.approx(2.0 * scale), \ + "unanimous-disagree comment must reach priority_metric with maximal extremity" + assert e_agree < e_disagree, \ + "extremity sign inverted: agree must be less extreme than disagree" + class TestD12PCAProjectComments: """`pca_project_cmnts` and `compute_comment_extremity` — Clojure parity.""" @@ -2082,7 +2136,18 @@ def test_pca_project_cmnts_shape(self): assert proj.shape == (4, 2) def test_pca_project_cmnts_formula(self): - """For comment i: proj[i] = -sqrt(n_cmnts) * (1 + center[i]) * [pc1[i], pc2[i]].""" + """Clojure-parity: proj[i] = sqrt(n_cmnts) * (AGREE - center[i]) * [pc1[i], pc2[i]]. + + Clojure (`pca-project-cmnts`, pca.clj:167-178) projects a synthetic vote + of `-1` because Clojure stays in raw-Postgres convention where AGREE = -1. + Delphi fits PCA in its OWN convention (AGREE = +1, via the + `postgres_vote_to_delphi` ingress flip), so the faithful port projects + the Delphi `AGREE` constant, not the literal -1. + + Expected is derived from the `AGREE` constant (NOT copied from the + implementation), so this catches a convention/sign regression instead of + rubber-stamping whatever the code currently computes. + """ center = np.array([0.1, 0.2, 0.3, 0.4]) comps = np.array([[1.0, 0.5, -0.5, 0.0], [0.0, 0.5, 0.5, 1.0]]) @@ -2090,7 +2155,7 @@ def test_pca_project_cmnts_formula(self): n_cmnts = 4 scale = np.sqrt(n_cmnts) for i in range(n_cmnts): - expected = -scale * (1 + center[i]) * comps[:, i] + expected = scale * (AGREE - center[i]) * comps[:, i] assert np.allclose(proj[i], expected), \ f"proj[{i}] = {proj[i]} vs expected {expected}" @@ -2114,6 +2179,33 @@ def test_compute_comment_extremity_empty(self): ext = compute_comment_extremity(np.zeros((0, 2))) assert ext.shape == (0,) + def test_extremity_sign_agree_low_disagree_high(self): + """Semantic guard on the convention translation (not the formula itself). + + In Delphi convention (AGREE = +1) the PCA center of a near-unanimous + AGREE comment → +1, and of a near-unanimous DISAGREE comment → -1. + Clojure-parity extremity is the L2 norm of `(AGREE - center) * pc`: + + unanimous AGREE (center → +1) ⇒ |AGREE - center| → 0 ⇒ extremity → 0 + unanimous DISAGREE (center → -1) ⇒ |AGREE - center| → 2 ⇒ extremity → max + + The pre-fix code used the untranslated Clojure literal `-1` + (`-scale*(1+center)`), which INVERTS this — a comment everyone agrees on + would read as maximally extreme. This test pins the direction and would + fail (agree > disagree) under that bug. + """ + # comps: pc1 = 1 for both comments, pc2 = 0 ⇒ extremity == |coef|. + center = np.array([1.0, -1.0]) # col 0 = agree pole, col 1 = disagree pole + comps = np.array([[1.0, 1.0], + [0.0, 0.0]]) + ext = compute_comment_extremity(pca_project_cmnts(center, comps)) + scale = np.sqrt(2) + assert ext[0] == pytest.approx(0.0, abs=1e-9), \ + "unanimous-agree comment must have extremity ~0" + assert ext[1] == pytest.approx(2.0 * scale), \ + "unanimous-disagree comment must have maximal extremity" + assert ext[0] < ext[1], "agree must be LESS extreme than disagree (sign check)" + class TestD12PriorityMetrics: """`importance_metric` and `priority_metric` — Clojure parity."""