Skip to content
Draft
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
50 changes: 31 additions & 19 deletions delphi/polismath/conversation/conversation.py
Original file line number Diff line number Diff line change
Expand Up @@ -489,17 +489,26 @@ def _apply_moderation(self) -> None:
"""
Apply moderation settings to create filtered rating matrix.

Matches Clojure behavior (named_matrix.clj:214-230):
- Moderated-out participants are removed (rows dropped)
- Moderated-out comments are ZEROED OUT, not removed — the column
stays in the matrix with all values set to 0. This preserves
matrix structure so that tids, column indices, and dimensions
match between Python and Clojure.
Comment moderation matches Clojure (named_matrix.clj:214-230):
moderated-out comments are ZEROED OUT, not removed — the column stays
in the matrix with all values set to 0. This preserves matrix
structure so that tids, column indices, and dimensions match between
Python and Clojure.

Participant bans (mod_out_ptpts) are a Python-only feature: the
Clojure worker's ingest path has no participants.mod filter, so
banned participants keep influencing every downstream node
(CLOJURE_QUIRKS Q1). 'clojure-legacy' mode replicates that leak —
the set is stored but NOT applied; 'improved' mode drops the rows.
"""
# Filter out moderated participants (remove rows).
# Filter out banned participants (remove rows) — improved mode only;
# legacy mode leaks like Clojure (Q1).
# Preserve raw_rating_mat row order (vote encounter order) — see
# update_votes() comment on why row order matters for Clojure parity.
keep_ptpts = [p for p in self.raw_rating_mat.index if p not in self.mod_out_ptpts]
if resolve_engine_mode() == ENGINE_MODE_LEGACY:
keep_ptpts = list(self.raw_rating_mat.index)
else:
keep_ptpts = [p for p in self.raw_rating_mat.index if p not in self.mod_out_ptpts]
self.rating_mat = self.raw_rating_mat.loc[keep_ptpts].copy()
Comment thread
jucor marked this conversation as resolved.

# Zero out moderated-out comments (keep columns, set values to 0)
Expand Down Expand Up @@ -1723,10 +1732,13 @@ def _compute_user_vote_counts(self) -> Dict[str, int]:
import time
start_time = time.time()
# raw_rating_mat for the COLUMN view (preserves moderated-out comments — D15
# parity), but filtered to rating_mat.index for the ROW view so moderated-out
# *participants* (mod_out_ptpts, dropped by _apply_moderation) don't leak
# into vote counts. Both filters together give the moderation-applied state
# with un-zeroed values, matching what Clojure produces.
# parity), but filtered to rating_mat.index for the ROW view so banned
# participants (mod_out_ptpts, dropped by _apply_moderation in improved
# mode) don't leak into vote counts. In clojure-legacy mode
# rating_mat.index keeps banned rows (Q1 leak replication), so this
# matches Clojure's unfiltered user-vote-counts there. Both filters
# together give the moderation-applied state with un-zeroed values,
# matching what Clojure produces.
mat = self.raw_rating_mat.loc[self.rating_mat.index]
logger.info(f"Starting _compute_user_vote_counts for {mat.shape[0]} participants")

Expand Down Expand Up @@ -1821,13 +1833,13 @@ def _get_in_conv_participants(self) -> Set[Any]:

# Legacy: carry forward the persisted in-conv set, then union the
# threshold set into it (Clojure `(into in-conv ...)`, conversation.clj:247-256).
# PRUNE the carry to participants still present in vote_counts first:
# vote_counts is keyed off rating_mat.index, which drops mod_out_ptpts
# (banned participants — a Python-only feature Clojure lacks). Without the
# intersection a participant banned AFTER being carried would linger in the
# set forever, inflating the size check so the greedy floor never re-fires
# to top the actually-clustered pool (proj ∩ in_conv) back up, and growing
# the carry unboundedly. Clojure-parity is unaffected (no ban feature there).
# The intersection with vote_counts is belt-and-braces: since the Q1
# ban-leak replication, legacy-mode rating_mat keeps banned rows, so
# vote_counts covers every carried pid and the intersection is inert
# (append-only votes mean a counted pid can never vanish). It stays as
# defense against any future row-view change re-opening the stale-carry
# trap #2623's T1 fixed (a carried pid missing from vote_counts would
# inflate the size check so the greedy floor never re-fires).
in_conv = (set(self.in_conv) & set(vote_counts.keys())) | threshold_set

# Greedy floor (conversation.clj:259-268): if under 15, admit the top
Expand Down
38 changes: 17 additions & 21 deletions delphi/tests/test_in_conv_greedy_carry.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,34 +151,30 @@ def test_qualifier_stays_in_across_ticks(self, monkeypatch, mode):
assert 'H0' in _clustered_pids(conv)


class TestCarryPruneOnParticipantBan:
"""T1 (#2623): a participant carried in self.in_conv and then BANNED
(mod_out_ptpts, a Python-only feature Clojure lacks) must be pruned from the
carry so the greedy floor tops the pool back up. Before the fix, the raw
carried set (`set(self.in_conv) | threshold_set`) still counted the banned
pids, so the size check saw a stale 15 while the actually-clustered pool
(proj ∩ in_conv, which excludes banned) had dropped below the floor — and
nothing topped it up. Legacy-mode-only; improved mode has no carry."""

def test_ban_after_carry_tops_floor_back_up(self, monkeypatch):
class TestCarryUnderParticipantBan:
"""Q1 leak replication (2026-07-22) SUPERSEDES #2623's T1 scenario: in
clojure-legacy mode a ban is stored but NOT applied (Clojure's worker never
honored participants.mod = -1), so banning can no longer shrink the
legacy-mode clustering pool and the stale-carry trap T1 fixed cannot arise.
The vote_counts intersection in _get_in_conv_participants stays as
belt-and-braces (see its comment). This test pins the new semantics:
carry and clustering are ban-invariant in legacy mode."""

def test_ban_after_carry_changes_nothing(self, monkeypatch):
_mode(monkeypatch, 'clojure-legacy')
# Tick 1: greedy floor fills to 15 (H0,H1 + L0..L12) and persists them.
conv = Conversation('ban').update_votes(_votes(_TICK1_SPECS))
assert len(conv.in_conv) == 15
banned = {'L0', 'L1', 'L2', 'L3', 'L4'}
assert banned.issubset(conv.in_conv) # all 5 are carried greedy admits

# Ban 5 of the 15 carried participants. vote_counts (from rating_mat.index)
# now excludes them, so the effective pool drops to 10 and the floor must
# re-admit 5 more of the remaining lows (L13..L19) to reach 15.
# Ban 5 of the 15 carried participants. Q1 leak: the set is stored but
# the pool, carry and clustering are unchanged — exactly as if Clojure
# had processed the same stream.
conv2 = conv.update_moderation({'mod_out_ptpts': list(banned)})

assert conv2.mod_out_ptpts == banned # stored ...
clustered = _clustered_pids(conv2)
# FIX: pruned carry -> floor re-fires -> back up to 15.
# BEFORE FIX: stale carry keeps count at 15 (floor never fires), but the
# banned rows aren't clustered -> only 10 clustered.
assert len(clustered) == 15
assert not (banned & clustered) # banned never clustered
assert not (banned & conv2.in_conv) # banned pruned from carry (no stale growth)
assert {'H0', 'H1'}.issubset(clustered) # qualifiers retained
assert len(conv2.in_conv) == 15
assert banned.issubset(clustered) # ... but still clustered
assert conv2.in_conv == conv.in_conv # carry untouched
assert len(clustered) == 15 # pool unchanged, floor idle
103 changes: 103 additions & 0 deletions delphi/tests/test_mod_ptpt_leak_parity.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
#!/usr/bin/env python3
"""
Participant-ban (participants.mod = -1) leak replication — CLOJURE_QUIRKS Q1.

The Clojure math worker NEVER honored participant bans: its ingest path has no
participants.mod filter, so mod_out_ptpts never reaches the conv and banned
participants keep influencing user-vote-counts, in-conv, PCA, clustering and
repness. Python gained a real ban feature (mod_out_ptpts row drop in
_apply_moderation, 2026-06-10) — correct, but a certification divergence.

Per Q1: 'clojure-legacy' mode must LEAK the ban exactly like Clojure (rows
kept everywhere); 'improved' mode keeps the fix. This supersedes the legacy-
mode premise of #2623's TestCarryPruneOnParticipantBan (banning can no longer
shrink the legacy clustering pool, so the carry-prune scenario cannot arise).
"""

import os
import sys

import pytest

sys.path.append(os.path.abspath(os.path.dirname(__file__)))

from polismath.pca_kmeans_rep.pca import PCA_IMPL_ENV_VAR
from polismath.utils.engine_mode import ENGINE_MODE_ENV_VAR
from polismath.conversation.conversation import Conversation


N_CMTS = 8


def _bloc_votes():
"""Two blocs of 6, everyone votes all comments (all above threshold)."""
votes = []
for i in range(6):
for t in range(N_CMTS):
votes.append({'pid': f'a{i}', 'tid': f'c{t}',
'vote': 1.0 if t < 4 else -1.0})
votes.append({'pid': f'b{i}', 'tid': f'c{t}',
'vote': -1.0 if t < 4 else 1.0})
return {'votes': votes}


def _clustered_pids(conv):
return {m for bc in conv.base_clusters for m in bc['members']}


@pytest.fixture
def legacy_mode(monkeypatch):
monkeypatch.delenv(PCA_IMPL_ENV_VAR, raising=False)
monkeypatch.setenv(ENGINE_MODE_ENV_VAR, 'clojure-legacy')


@pytest.fixture
def improved_mode(monkeypatch):
monkeypatch.delenv(PCA_IMPL_ENV_VAR, raising=False)
monkeypatch.setenv(ENGINE_MODE_ENV_VAR, 'improved')


class TestLegacyBanLeak:

def test_banned_participant_rows_kept(self, legacy_mode):
conv = Conversation('leak').update_votes(_bloc_votes())
conv = conv.update_moderation({'mod_out_ptpts': ['a0', 'b0']})
# The ban is STORED (payload bookkeeping unchanged) ...
assert conv.mod_out_ptpts == {'a0', 'b0'}
# ... but NOT applied: Clojure's worker never drops banned rows.
assert 'a0' in conv.rating_mat.index
assert 'b0' in conv.rating_mat.index

def test_banned_participant_still_clustered(self, legacy_mode):
conv = Conversation('leak').update_votes(_bloc_votes())
conv = conv.update_moderation({'mod_out_ptpts': ['a0', 'b0']})
clustered = _clustered_pids(conv)
assert {'a0', 'b0'}.issubset(clustered)
# And they stay through a subsequent vote tick.
conv = conv.update_votes({'votes': [
{'pid': 'a1', 'tid': 'c0', 'vote': 1.0}]})
assert {'a0', 'b0'}.issubset(_clustered_pids(conv))

def test_banned_participant_counted_in_user_vote_counts(self, legacy_mode):
conv = Conversation('leak').update_votes(_bloc_votes())
conv = conv.update_moderation({'mod_out_ptpts': ['a0']})
assert conv._compute_user_vote_counts().get('a0') == N_CMTS

def test_banned_participant_stays_in_conv(self, legacy_mode):
conv = Conversation('leak').update_votes(_bloc_votes())
conv = conv.update_moderation({'mod_out_ptpts': ['a0']})
assert 'a0' in conv.in_conv


class TestImprovedBanKept:
"""Improved mode keeps the real ban feature byte-for-byte."""

def test_banned_participant_dropped_and_not_clustered(self, improved_mode):
conv = Conversation('leak').update_votes(_bloc_votes())
conv = conv.update_moderation({'mod_out_ptpts': ['a0']})
assert 'a0' not in conv.rating_mat.index
assert 'a0' not in _clustered_pids(conv)


if __name__ == '__main__':
pytest.main([__file__, '-v'])
Loading