Skip to content

Commit 0a8ca66

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 dbe27cf commit 0a8ca66

5 files changed

Lines changed: 563 additions & 168 deletions

File tree

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

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -712,6 +712,65 @@ 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`, `votes-base`, `_compute_vote_stats`) is routed
738+
through `raw_rating_mat` so the moderation-zeroed values in `rating_mat` don't
739+
inflate counts. This matches Clojure's `conversation.clj:220-228` (uses
740+
`raw-rating-mat` for `:user-vote-counts`) and `:593-600` (uses `raw-rating-mat`
741+
for `:votes-base`). See the "Audit + Recovery (2026-06-09)" session at the
742+
bottom of this journal for the downstream-fix landing details — the audit
743+
caught and resolved the earlier-claimed `rating_mat` routing.
744+
745+
### Tests
746+
747+
**New synthetic tests** (`TestD15SyntheticModeration`, 5 tests):
748+
- `test_zeroing_preserves_columns` — moderated columns still present
749+
- `test_zeroed_columns_are_all_zero` — moderated column values are 0.0
750+
- `test_non_moderated_columns_unchanged` — other columns retain original values
751+
- `test_empty_moderation_no_change` — no-op when no moderation
752+
- `test_moderate_nonexistent_tid` — graceful handling of unknown tids
753+
754+
**Enhanced real-data tests** (`TestD15ModerationHandling`, 2 tests):
755+
- `test_moderated_comments_zeroed_not_removed` — applies mod-out from Clojure blob, checks column count and zeroed values
756+
- `test_tids_include_moderated` — verifies moderated tids remain in rating_mat columns
757+
758+
**Updated existing tests**:
759+
- `test_conversation.py::test_moderation` — updated to expect zeroed columns
760+
- `test_conversation.py::test_update_moderation` — same
761+
- `test_discrepancy_fixes.py::TestD2cVoteCountSource::test_n_cmts_includes_moderated_out_comments` — updated comment count assertion
762+
763+
### Test results
764+
765+
- Public datasets: **328 passed, 0 failed, 6 skipped, 56 xfailed**
766+
- Private datasets: 13 failures — all **pre-existing** (golden snapshot staleness from earlier fixes, not D15-related). Verified by running parent branch.
767+
768+
### What's next
769+
770+
- D12 (comment priorities) or D1/D1b (PCA sign flips) — per plan ordering
771+
772+
---
773+
715774
## Notes for Future Sessions
716775

717776
- 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

0 commit comments

Comments
 (0)