Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions delphi/docs/CLJ-PARITY-FIXES-JOURNAL.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
2 changes: 1 addition & 1 deletion delphi/docs/PLAN_DISCREPANCY_FIXES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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). |
Comment thread
jucor marked this conversation as resolved.
| 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** ✓ |
Expand Down
28 changes: 21 additions & 7 deletions delphi/polismath/pca_kmeans_rep/pca.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)


Expand Down Expand Up @@ -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.

Expand All @@ -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)


Expand Down
9 changes: 9 additions & 0 deletions delphi/polismath/regression/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
96 changes: 94 additions & 2 deletions delphi/tests/test_discrepancy_fixes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand All @@ -2082,15 +2136,26 @@ 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]])
proj = pca_project_cmnts(center, comps)
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}"

Expand All @@ -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."""
Expand Down
Loading