Skip to content

Commit 7baf680

Browse files
committed
Fix D15: match Clojure moderation handling (zero out columns, don't remove)
## Summary Python's `_apply_moderation()` removed moderated-out comment columns entirely from `rating_mat`. Clojure's `zero-out-columns` (named_matrix.clj:214-230) sets all values in moderated columns to 0, preserving matrix structure. This fix changes Python to match: - Moderated-out comment columns are **zeroed** (values set to 0.0), not removed - `rating_mat` retains the same column count as `raw_rating_mat` - Moderated-out participants (rows) are still removed — unchanged ### Why zeroing matters - **Matrix dimensions**: Clojure's `rating-mat` has the same shape as `raw-rating-mat`. Downstream code (PCA, repness) processes the same-shaped matrix. - **tids list**: Column indices stay stable. Consumers depend on this. - **Practical impact**: Zeroed columns have no signal (na=0, nd=0), so they fail significance tests and are excluded from repness/consensus. PCA sees zero variance. ## Changes - `conversation.py`: `_apply_moderation()` — zero out columns instead of removing - `test_discrepancy_fixes.py`: 5 new synthetic tests + 2 enhanced real-data tests - `test_conversation.py`: Updated to expect zeroed columns ## Test plan - [x] Synthetic tests: zeroing preserves columns, values are 0, non-moderated unchanged - [x] Real-data test: biodiversity-incremental (169 mod-out comments) - [x] Full public test suite: 328 passed, 0 failed - [x] TDD cycle: RED (2 failures) → GREEN (all pass) 🤖 Generated with [Claude Code](https://claude.com/claude-code) ## Squashed commits - Fix D15: match Clojure moderation handling (zero out columns, don't remove) commit-id:c3450b9a
1 parent d12b44b commit 7baf680

5 files changed

Lines changed: 255 additions & 29 deletions

File tree

delphi/docs/CLJ-PARITY-FIXES-JOURNAL.md

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -712,6 +712,60 @@ to create a new worktree. If yes, provide a prompt they can use to start that se
712712
713713
---
714714

715+
## Session: Fix D15 — Moderation Handling (2026-03-16)
716+
717+
### Branch: `jc/clj-parity-d15-moderation-handling-zeros-vs-removes`
718+
719+
### What was done
720+
721+
Fixed D15: Python now zeros out moderated-out comment columns instead of removing them,
722+
matching Clojure's `zero-out-columns` behavior (named_matrix.clj:214-230).
723+
724+
**The discrepancy**: Python's `_apply_moderation()` removed moderated-out columns from
725+
`rating_mat` entirely (`raw_rating_mat.loc[keep_ptpts, keep_comments]`). Clojure zeros
726+
them out (`matrix/set-column m' i 0`), preserving matrix structure.
727+
728+
**The fix**: Changed `_apply_moderation()` to:
729+
1. Still remove moderated-out participants (rows) — unchanged
730+
2. Zero out moderated-out comment columns instead of removing them
731+
3. `rating_mat` now has the same column count as `raw_rating_mat`
732+
733+
**Impact on downstream**:
734+
- `tids` output now includes moderated-out tids (matching Clojure)
735+
- PCA: zeroed columns contribute nothing to variance, so PCA results are effectively identical
736+
- Repness: zeroed columns get na=0, nd=0, failing significance — effectively excluded
737+
- Vote counting: `user-vote-counts` in `to_math_blob()` uses `rating_mat`, so moderated
738+
columns now count as "pass" votes (matching Clojure's behavior with zeroed columns)
739+
740+
### Tests
741+
742+
**New synthetic tests** (`TestD15SyntheticModeration`, 5 tests):
743+
- `test_zeroing_preserves_columns` — moderated columns still present
744+
- `test_zeroed_columns_are_all_zero` — moderated column values are 0.0
745+
- `test_non_moderated_columns_unchanged` — other columns retain original values
746+
- `test_empty_moderation_no_change` — no-op when no moderation
747+
- `test_moderate_nonexistent_tid` — graceful handling of unknown tids
748+
749+
**Enhanced real-data tests** (`TestD15ModerationHandling`, 2 tests):
750+
- `test_moderated_comments_zeroed_not_removed` — applies mod-out from Clojure blob, checks column count and zeroed values
751+
- `test_tids_include_moderated` — verifies moderated tids remain in rating_mat columns
752+
753+
**Updated existing tests**:
754+
- `test_conversation.py::test_moderation` — updated to expect zeroed columns
755+
- `test_conversation.py::test_update_moderation` — same
756+
- `test_discrepancy_fixes.py::TestD2cVoteCountSource::test_n_cmts_includes_moderated_out_comments` — updated comment count assertion
757+
758+
### Test results
759+
760+
- Public datasets: **328 passed, 0 failed, 6 skipped, 56 xfailed**
761+
- Private datasets: 13 failures — all **pre-existing** (golden snapshot staleness from earlier fixes, not D15-related). Verified by running parent branch.
762+
763+
### What's next
764+
765+
- D12 (comment priorities) or D1/D1b (PCA sign flips) — per plan ordering
766+
767+
---
768+
715769
## Notes for Future Sessions
716770

717771
- Private datasets are in `delphi/real_data/.local/` (separate git repo, linked via `link-to-polis-worktree.sh`)

delphi/docs/PLAN_DISCREPANCY_FIXES.md

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,9 @@ Because this work will span multiple Claude Code sessions, we maintain:
4747
### Testing Principles
4848

4949
- **Granular tests per discrepancy**: Not just overall regression — each fix gets its own targeted test checking the specific aspect it addresses. Multiple discrepancies may affect `test_basic_outputs`; we need to see incremental improvement per fix.
50+
- **Clojure blob comparison is MANDATORY**: Every fix PR must include tests that compare Python output to actual Clojure math blob values — not just formula re-implementation tests (which are tautological: they only verify our code matches our reading of the Clojure, not that it matches Clojure's actual output). The Clojure blob is the ground truth oracle.
51+
- **Stage isolation via blob injection**: Since upstream stages (PCA, clustering) may not match between Python and Clojure, tests must inject Clojure blob values as inputs to the stage being tested, then compare outputs. For example: to test `prop_test` (D5), extract `n-success` and `n-trials` from the Clojure blob's `repness` entries, feed them to Python's `prop_test()`, and compare the result to the blob's `p-test`. This isolates each stage from upstream divergence.
52+
- **Blob fields available for injection/comparison**: The Clojure cold-start blob provides per-group repness entries with: `n-success` (=na), `n-trials` (=ns), `p-success` (=pa), `p-test` (=pat), `repness` (=ra), `repness-test` (=rat), `repful-for`, `best-agree`, `tid`. Also: `group-clusters` (memberships), `group-votes` (per-group vote counts), `consensus` (selected consensus comments), `comment-priorities` (per-tid priority values), `in-conv` (participant list).
5053
- **Targeted pipeline-stage tests**: For D2/D3 (participant filtering, clustering), check in-conv count, cluster count, and cluster memberships against Clojure blob. For D12, check comment-priorities against Clojure blob.
5154
- **All datasets, not just biodiversity**: Every fix must pass on ALL datasets. biodiversity is just one reference among many.
5255
- **Synthetic edge-case tests**: Every time we discover an edge case specific to one conversation, extract it into a synthetic unit test with made-up data (never real data from private datasets). These run fast and document the intent clearly.
@@ -441,6 +444,38 @@ By this point, we should have good test coverage from all the per-discrepancy te
441444

442445
---
443446

447+
### Investigation: Cold-Start K Divergence (after D15, before D12)
448+
449+
**Prerequisite**: All cold-start-relevant upstream fixes complete: D2/D2c/D2b (in-conv,
450+
vote counts, sort order), D15 (moderation handling). Note: D1 (PCA sign flips) only
451+
affects incremental updates — on cold start there are no previous components to align to.
452+
453+
After D15, the rating matrix construction, in-conv filtering, and PCA inputs should all
454+
match Clojure. Both implementations use silhouette for k-selection. Yet on vw, Python
455+
selects k=4 while Clojure selects k=2.
456+
457+
**Investigation steps**:
458+
459+
1. **PCA component comparison**: Feed the same rating matrix to both sklearn TruncatedSVD
460+
and a Python reimplementation of Clojure's power iteration. Quantify divergence
461+
(cosine similarity per component, Frobenius norm).
462+
2. **Projection comparison**: Inject Clojure blob's PCA components into Python's
463+
clustering path. Does k now match?
464+
3. **Base-cluster comparison**: Given the same projections, compare k-means centroids
465+
and member assignments. Check initialization (Clojure uses first-k-distinct centers
466+
from base clusters — does Python match?).
467+
4. **Silhouette score comparison**: Given the same base clusters, compare per-k
468+
silhouette scores. Are the scores close but the winner differs?
469+
5. **All datasets**: Run on all datasets with cold-start blobs, not just vw.
470+
471+
**Outcome**: Either (a) identify a fixable discrepancy that makes k match, or
472+
(b) document the inherent numerical divergence between sklearn SVD and Clojure
473+
power iteration, and establish tolerance bounds for k agreement in tests.
474+
475+
See `delphi/docs/HANDOFF_K_DIVERGENCE_INVESTIGATION.md` for detailed context.
476+
477+
---
478+
444479
### Explicitly Deferred
445480

446481
- **D13 — Subgroup Clustering**: Not implemented in Python, never used by TypeScript consumers. No fix needed.
@@ -470,14 +505,15 @@ By this point, we should have good test coverage from all the per-discrepancy te
470505
| D5 | Proportion test | **PR 4** || **DONE**|
471506
| D6 | Two-proportion test | **PR 5** || **DONE**|
472507
| D7 | Repness metric | PR 6 || **DONE**|
473-
| D8 | Finalize cmt stats | PR 7 || Fix |
508+
| D8 | Finalize cmt stats | PR 7 || **DONE** |
474509
| D9 | Z-score thresholds | **PR 3** | **#2446** | **DONE**|
475510
| D10 | Rep comment selection | PR 8 || Fix (with legacy env var) |
476511
| D11 | Consensus selection | PR 9 || Fix (with legacy env var) |
477512
| D12 | Comment priorities | PR 11 || Fix (implement from scratch) |
478513
| D13 | Subgroup clustering ||| **Deferred** (unused) |
479514
| D14 | Large conv optimization ||| **Deferred** (Python fast enough) |
480-
| D15 | Moderation handling | PR 12 || Fix |
515+
| D15 | Moderation handling | PR 12 || **DONE**|
516+
| K-inv | Cold-start k divergence | (investigation) || Branch off D15 (D2+D15 done, clustering independent of repness) |
481517
| Replay | Replay infrastructure (A/B/C) ||| NOT BUILT — D3/D1 used synthetic tests only. Needed for incremental blob comparison. |
482518

483519
### Non-discrepancy PRs in the stack

delphi/polismath/conversation/conversation.py

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -296,15 +296,23 @@ def update_votes(self,
296296
def _apply_moderation(self) -> None:
297297
"""
298298
Apply moderation settings to create filtered rating matrix.
299+
300+
Matches Clojure behavior (named_matrix.clj:214-230):
301+
- Moderated-out participants are removed (rows dropped)
302+
- Moderated-out comments are ZEROED OUT, not removed — the column
303+
stays in the matrix with all values set to 0. This preserves
304+
matrix structure so that tids, column indices, and dimensions
305+
match between Python and Clojure.
299306
"""
300-
# Filter out moderated participants and comments, and keep them sorted!
301-
# Note: set operations are unordered, hence the extra sort.
302-
# Natural sort: preserves types and sorts numerically when possible
307+
# Filter out moderated participants (remove rows)
303308
keep_ptpts = natsorted(list(set(self.raw_rating_mat.index) - set(self.mod_out_ptpts)))
304-
keep_comments = natsorted(list(set(self.raw_rating_mat.columns) - set(self.mod_out_tids)))
305-
306-
# Create filtered matrix
307-
self.rating_mat = self.raw_rating_mat.loc[keep_ptpts, keep_comments]
309+
self.rating_mat = self.raw_rating_mat.loc[keep_ptpts].copy()
310+
311+
# Zero out moderated-out comments (keep columns, set values to 0)
312+
# Clojure: (matrix/set-column m' i 0) — zeroes the column
313+
mod_cols = [c for c in self.mod_out_tids if c in self.rating_mat.columns]
314+
if mod_cols:
315+
self.rating_mat[mod_cols] = 0.0
308316

309317
def _compute_vote_stats(self) -> None:
310318
"""

delphi/tests/test_conversation.py

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -408,10 +408,13 @@ def test_moderation(self):
408408
assert 'c2' in moderated_conv.mod_out_tids
409409
assert 'p3' in moderated_conv.mod_out_ptpts
410410

411-
# Check filtered rating matrix
412-
assert 'c2' not in moderated_conv.rating_mat.columns
413-
assert 'p3' not in moderated_conv.rating_mat.index
414-
411+
# Check filtered rating matrix:
412+
# - Moderated-out comments are ZEROED, not removed (D15 fix)
413+
# - Moderated-out participants are still removed (rows dropped)
414+
assert 'c2' in moderated_conv.rating_mat.columns # column kept
415+
assert (moderated_conv.rating_mat['c2'] == 0.0).all() # but zeroed
416+
assert 'p3' not in moderated_conv.rating_mat.index # participant removed
417+
415418
# Raw matrix should still have all data
416419
assert 'c2' in moderated_conv.raw_rating_mat.columns
417420
assert 'p3' in moderated_conv.raw_rating_mat.index
@@ -605,9 +608,10 @@ def test_update_moderation(self):
605608

606609
conv = manager.update_moderation('test_conv', moderation)
607610

608-
# Check moderation was applied
611+
# Check moderation was applied: column kept but zeroed (D15 fix)
609612
assert 'c2' in conv.mod_out_tids
610-
assert 'c2' not in conv.rating_mat.columns
613+
assert 'c2' in conv.rating_mat.columns
614+
assert (conv.rating_mat['c2'] == 0.0).all()
611615

612616
# Suppress sklearn PCA warning: test uses minimal data (2 participants, 2 comments)
613617
# which can have zero variance. The test validates that recompute runs, not PCA quality.

delphi/tests/test_discrepancy_fixes.py

Lines changed: 138 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -289,12 +289,13 @@ def test_n_cmts_includes_moderated_out_comments(self):
289289
participant_votes={0: list(range(10))},
290290
)
291291

292-
# raw_rating_mat has all columns; rating_mat has only non-moderated-out
292+
# Both raw_rating_mat and rating_mat have all columns (D15 fix:
293+
# moderated-out columns are zeroed, not removed)
293294
n_cmts_raw = len(conv.raw_rating_mat.columns)
294295
n_cmts_filtered = len(conv.rating_mat.columns)
295296

296297
assert n_cmts_raw == 10, f"raw_rating_mat should have 10 columns, got {n_cmts_raw}"
297-
assert n_cmts_filtered == 5, f"rating_mat should have 5 columns, got {n_cmts_filtered}"
298+
assert n_cmts_filtered == 10, f"rating_mat should keep all 10 columns (zeroed, not removed), got {n_cmts_filtered}"
298299

299300
# The threshold used by _get_in_conv_participants should be min(7, 10) = 7,
300301
# not min(7, 5) = 5. Verify indirectly: participant with exactly 6 votes
@@ -1146,29 +1147,152 @@ class TestD15ModerationHandling:
11461147
"""
11471148
D15: Python removes moderated comments entirely from matrix.
11481149
Clojure zeros them out (keeps structure, sets values to 0).
1150+
1151+
Clojure behavior (named_matrix.clj:214-230):
1152+
zero-out-columns sets all values in moderated columns to 0,
1153+
preserving the matrix structure (same number of columns).
1154+
1155+
Python should match: _apply_moderation() must zero out moderated
1156+
columns rather than removing them, so that:
1157+
- rating_mat.columns includes moderated tids (zeroed)
1158+
- tids output includes moderated tids
1159+
- Matrix dimensions match Clojure
11491160
"""
11501161

11511162
def test_moderated_comments_zeroed_not_removed(self, conv, clojure_blob, dataset_name):
11521163
"""
1153-
Moderated comments should be zeroed out, not removed, if any exist.
1154-
1155-
Note: This test only applies when the dataset has moderated comments.
1164+
After applying moderation, rating_mat should still have all columns.
1165+
Moderated columns should be zeroed, not removed.
11561166
"""
1157-
# Check if Clojure blob has mod-out comments
1158-
mod_out = clojure_blob.get('mod-out', [])
1167+
mod_out = clojure_blob.get('mod-out') or []
11591168
if not mod_out:
11601169
pytest.skip(f"[{dataset_name}] No moderated comments in this dataset")
11611170

1162-
# If there ARE moderated comments, check Python's handling
1163-
n_cols_python = len(conv.rating_mat.columns)
1171+
# Apply moderation from the Clojure blob to the Python conversation
1172+
mod_conv = conv.update_moderation(
1173+
{'mod_out_tids': mod_out},
1174+
recompute=False,
1175+
)
1176+
1177+
n_cols_python = len(mod_conv.rating_mat.columns)
1178+
n_cols_raw = len(mod_conv.raw_rating_mat.columns)
11641179
n_tids_clojure = len(clojure_blob.get('tids', []))
11651180

1166-
print(f"[{dataset_name}] Moderated comments: {len(mod_out)}")
1167-
print(f"[{dataset_name}] Python matrix columns: {n_cols_python}, Clojure tids: {n_tids_clojure}")
1181+
print(f"[{dataset_name}] mod-out: {len(mod_out)}")
1182+
print(f"[{dataset_name}] Python rating_mat cols: {n_cols_python}, raw cols: {n_cols_raw}")
1183+
print(f"[{dataset_name}] Clojure tids: {n_tids_clojure}")
11681184

1169-
# Clojure keeps all tids (zeroed for mod-out), Python removes them
1170-
check.equal(n_cols_python, n_tids_clojure,
1171-
f"Matrix columns differ: Python={n_cols_python}, Clojure={n_tids_clojure} (mod-out={len(mod_out)})")
1185+
# After zeroing (not removing), column count should match raw matrix
1186+
check.equal(
1187+
n_cols_python, n_cols_raw,
1188+
f"rating_mat should keep all columns (zeroed, not removed): "
1189+
f"got {n_cols_python}, expected {n_cols_raw}"
1190+
)
1191+
1192+
# Moderated columns should be all zeros (not NaN, not original values)
1193+
for tid in mod_out:
1194+
if tid in mod_conv.rating_mat.columns:
1195+
col_values = mod_conv.rating_mat[tid].values
1196+
check.is_true(
1197+
np.all(col_values == 0.0),
1198+
f"Moderated tid {tid} should be all zeros, "
1199+
f"got non-zero values: {col_values[col_values != 0.0][:5]}"
1200+
)
1201+
1202+
def test_tids_include_moderated(self, conv, clojure_blob, dataset_name):
1203+
"""The tids output should include moderated-out comments (matching Clojure)."""
1204+
mod_out = clojure_blob.get('mod-out') or []
1205+
if not mod_out:
1206+
pytest.skip(f"[{dataset_name}] No moderated comments in this dataset")
1207+
1208+
mod_conv = conv.update_moderation(
1209+
{'mod_out_tids': mod_out},
1210+
recompute=False,
1211+
)
1212+
1213+
# rating_mat.columns (used for tids output) should include moderated tids
1214+
for tid in mod_out:
1215+
if tid in mod_conv.raw_rating_mat.columns:
1216+
check.is_in(
1217+
tid, set(mod_conv.rating_mat.columns),
1218+
f"Moderated tid {tid} should still be in rating_mat columns"
1219+
)
1220+
1221+
1222+
class TestD15SyntheticModeration:
1223+
"""
1224+
Synthetic tests for D15 moderation handling.
1225+
1226+
Clojure zeros out moderated columns (named_matrix.clj:214-230).
1227+
Python must match: _apply_moderation() zeros columns, not removes them.
1228+
"""
1229+
1230+
def _make_conversation_with_moderation(self, mod_out_tids):
1231+
"""Create a small conversation and apply moderation."""
1232+
import pandas as pd
1233+
1234+
# 5 participants, 4 comments. Votes: agree=1, disagree=-1, pass=0, no vote=NaN
1235+
data = {
1236+
0: [1.0, -1.0, 1.0, np.nan, 0.0],
1237+
1: [-1.0, 1.0, 0.0, 1.0, -1.0],
1238+
2: [1.0, 1.0, -1.0, -1.0, 1.0],
1239+
3: [np.nan, 0.0, 1.0, 1.0, -1.0],
1240+
}
1241+
votes_df = pd.DataFrame(data, index=[0, 1, 2, 3, 4])
1242+
1243+
conv = Conversation("synthetic_d15")
1244+
conv.raw_rating_mat = votes_df.copy()
1245+
conv.rating_mat = votes_df.copy()
1246+
conv.participant_count, conv.comment_count = votes_df.shape
1247+
1248+
# Apply moderation
1249+
conv.mod_out_tids = set(mod_out_tids)
1250+
conv._apply_moderation()
1251+
return conv
1252+
1253+
def test_zeroing_preserves_columns(self):
1254+
"""Moderated columns should still be present in rating_mat."""
1255+
conv = self._make_conversation_with_moderation(mod_out_tids=[1, 3])
1256+
1257+
# All 4 columns should still be present
1258+
assert len(conv.rating_mat.columns) == 4, (
1259+
f"Expected 4 columns, got {len(conv.rating_mat.columns)}: "
1260+
f"moderated columns should be zeroed, not removed"
1261+
)
1262+
assert set(conv.rating_mat.columns) == {0, 1, 2, 3}
1263+
1264+
def test_zeroed_columns_are_all_zero(self):
1265+
"""Moderated columns should have all values set to 0.0."""
1266+
conv = self._make_conversation_with_moderation(mod_out_tids=[1, 3])
1267+
1268+
for tid in [1, 3]:
1269+
col = conv.rating_mat[tid].values
1270+
assert np.all(col == 0.0), (
1271+
f"Moderated column {tid} should be all zeros, got {col}"
1272+
)
1273+
1274+
def test_non_moderated_columns_unchanged(self):
1275+
"""Non-moderated columns should retain their original values."""
1276+
conv = self._make_conversation_with_moderation(mod_out_tids=[1])
1277+
1278+
# Column 0 should be unchanged: [1, -1, 1, NaN, 0]
1279+
col0 = conv.rating_mat[0].values
1280+
assert col0[0] == 1.0
1281+
assert col0[1] == -1.0
1282+
assert np.isnan(col0[3]) # NaN preserved for non-moderated
1283+
1284+
def test_empty_moderation_no_change(self):
1285+
"""No moderation should leave the matrix unchanged."""
1286+
conv = self._make_conversation_with_moderation(mod_out_tids=[])
1287+
assert len(conv.rating_mat.columns) == 4
1288+
1289+
def test_moderate_nonexistent_tid(self):
1290+
"""Moderating a tid that doesn't exist in the matrix should be a no-op."""
1291+
conv = self._make_conversation_with_moderation(mod_out_tids=[99])
1292+
# All columns preserved, no crash
1293+
assert len(conv.rating_mat.columns) == 4
1294+
# Original values intact
1295+
assert conv.rating_mat[0].values[0] == 1.0
11721296

11731297

11741298
# ============================================================================

0 commit comments

Comments
 (0)