[Stack 10/27] Fix D2: in-conv participant threshold + D2c vote count source

[jucor]

Summary

Stacked on #2420 (Per-discrepancy test infrastructure). Please review and merge #2420 first.
Next in stack: #2435 (Fix D4: pseudocount formula)

Fixes the in-conv participant threshold (D2), vote count source (D2c), and base-cluster sort order (D2b) to match Clojure. Adds monotonicity guard tests (D2d).

D2: In-conv threshold

• Before: threshold = 7 + sqrt(n_cmts) * 0.1 — increasingly restrictive for larger conversations (e.g., 8.8 for biodiversity's 314 comments)
• After: threshold = min(7, n_cmts) — matches Clojure exactly

D2b: Base-cluster sort order (from Copilot review)

• Before: Base clusters sorted by size (descending) with IDs reassigned — changes encounter order of centers fed into group-level k-means
• After: Keep k-means ID order, matching Clojure's (sort-by :id ...)

D2c: Vote count source (raw vs filtered matrix)

• Before: _compute_user_vote_counts and n_cmts used self.rating_mat (filtered — moderated-out comment columns removed). A participant who voted on 8 comments could drop to 5 visible votes after 3 comments were moderated-out, falling below threshold.
• After: Both use self.raw_rating_mat (includes all votes, even on moderated-out comments), matching Clojure's user-vote-counts (conversation.clj:217-225) which reads from raw-rating-mat.

D2d: In-conv monotonicity (design decision)

Python does full recompute from raw_rating_mat every time, so monotonicity ("once in, always in") is guaranteed without persistence — votes are immutable in PostgreSQL, so a participant's count never decreases. This is strictly better than Clojure's approach (which persists in-conv to math_main because it uses delta vote processing).

5 guard tests (T1-T5) document this invariant and warn that switching to delta processing would require persisting in-conv to DynamoDB (ref: #2358).

Impact

• biodiversity: 428 → 441 in-conv participants (now matches Clojure)
• Verified on 4 datasets with complete Clojure cold-start blobs

Incremental vs cold-start blob testing

D2 tests run against both cold-start and incremental Clojure blobs (infrastructure from #2420):

• Cold-start blobs are computed in one pass on the full dataset. The in-conv threshold min(7, n_cmts) is evaluated once with the final n_cmts. Python matches these exactly.
• Incremental blobs were built progressively as votes trickled in over the conversation's lifetime. The threshold was evaluated at each iteration with a smaller n_cmts, admitting a few extra participants during earlier iterations. The difference is tiny (1–2 participants).

D2 tests on incremental blobs are currently xfailed with an explanatory comment. Matching incremental behaviour exactly would require simulating the progressive threshold — tracked as future work under Replay Infrastructure.

Test results

253 passed, 5 skipped, 36 xfailed (0 failures)

Test plan

• D2 tests pass on all datasets with complete Clojure cold-start blobs
• D2c: 3 synthetic tests verify vote counts include moderated-out votes, n_cmts includes moderated-out comments, participants stay in-conv after moderation
• D2d: 5 monotonicity tests (basic across updates, survives moderation, worker restart + moderation, restart without new votes, mixed participants)
• D2 tests xfail on incremental blobs (with explanatory comments)
• Full test suite: 253 passed, 0 failures
• Golden snapshots re-recorded for affected datasets

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Aligns Python’s in-conv participant selection and clustering inputs with the legacy Clojure implementation (D2/D2b/D2c), and expands the regression test harness to handle multiple Clojure blob variants (incremental vs cold-start) while adding guard tests for monotonicity (D2d).

Changes:

• Update in-conv logic to use threshold = min(7, n_cmts) and compute both n_cmts + vote counts from raw_rating_mat (includes moderated-out comment votes).
• Preserve base-cluster encounter order by keeping k-means IDs (sort-by id, no size-based reordering / ID reassignment).
• Extend test infrastructure to parametrize over blob variants and add D2c/D2d synthetic/guard tests.

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
delphi/polismath/conversation/conversation.py	Implements D2/D2b/D2c logic changes (threshold, raw vote counts, base-cluster order) and adds rationale comments.
delphi/polismath/regression/datasets.py	Adds blob_type selection to get_dataset_files() and introduces get_blob_variants() discovery via _is_blob_filled().
delphi/polismath/regression/init.py	Exposes get_blob_variants from the regression package.
delphi/tests/conftest.py	Adds use_blobs support to use_discovered_datasets marker and introduces parse_dataset_blob_id().
delphi/tests/test_discrepancy_fixes.py	Converts D2 tests to blob-aware datasets, adds D2c vote-source tests and D2d monotonicity guard tests.
delphi/tests/test_legacy_clojure_regression.py	Converts legacy Clojure regression tests to run per dataset+blob variant with caching.
delphi/tests/test_legacy_repness_comparison.py	Updates legacy repness comparison to use blob-aware dataset IDs.
delphi/tests/test_repness_smoke.py	Removes an xfail on repness structure validation.
delphi/tests/test_conversation.py	Updates the in-test comment describing the vote threshold formula.
delphi/real_data/r6vbnhffkxbd7ifmfbdrd-vw/golden_snapshot.json	Re-recorded golden snapshot (timestamps, math_tick, and timing stats changed).
delphi/docs/PLAN_DISCREPANCY_FIXES.md	Marks D2/D2b/D2c/D2d as DONE and documents the monotonicity design note.
delphi/docs/CLJ-PARITY-FIXES-JOURNAL.md	Updates the parity-fix journal with session notes, rationale, and outcomes for D2c/D2d.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 18 out of 19 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

delphi/polismath/conversation/conversation.py:1291

• update_votes() explicitly preserves pid/tid types (they can be ints/strings), and the new synthetic tests also use int pids. _compute_user_vote_counts() and _get_in_conv_participants() are annotated as Dict[str, int] / Set[str], but they actually return keys of whatever type the matrix index uses. Adjust the type hints (e.g., Hashable/Any) to match runtime behavior so the annotations remain accurate.

    def _compute_user_vote_counts(self) -> Dict[str, int]:
        """
        Compute the number of votes per participant.

        Uses raw_rating_mat (not rating_mat) so that votes on moderated-out
        comments are still counted. This matches Clojure's user-vote-counts
        (conversation.clj:217-225) which reads from raw-rating-mat.
        Fix D2c: see PLAN_DISCREPANCY_FIXES.md.

        Returns:
            Dictionary mapping participant IDs to vote counts
        """
        import time
        start_time = time.time()
        mat = self.raw_rating_mat
        logger.info(f"Starting _compute_user_vote_counts for {mat.shape[0]} participants")

        vote_counts = {}

        # Use more efficient approach for large datasets
        if mat.shape[0] > 1000:
            # Create a mask of non-nan values across the entire matrix
            non_nan_mask = ~np.isnan(mat.values)

            # Sum across rows using vectorized operation
            row_sums = np.sum(non_nan_mask, axis=1)

            # Convert to dictionary
            for i, pid in enumerate(mat.index):
                if i < len(row_sums):
                    vote_counts[pid] = int(row_sums[i])
                else:
                    # Fallback if dimensions don't match
                    vote_counts[pid] = 0

            logger.info(f"Computed vote counts for {len(vote_counts)} participants using vectorized approach in {time.time() - start_time:.4f}s")
        else:
            # Original approach for smaller datasets
            for i, pid in enumerate(mat.index):
                # Get row of votes for this participant
                row = mat.values[i, :]

                # Count non-nan values
                count = np.sum(~np.isnan(row))

                # Store count
                vote_counts[pid] = int(count)

            logger.info(f"Computed vote counts for {len(vote_counts)} participants using original approach in {time.time() - start_time:.4f}s")

        return vote_counts

    def _get_in_conv_participants(self) -> Set[str]:
        """
        Get participants who have voted enough to be included in clustering.

        Matches Clojure's in-conv logic from conversation.clj lines 239-266.

        Threshold: participant must have voted on at least min(7, n_comments)
        comments (Clojure parity fix D2).

        Both vote counts and n_cmts use raw_rating_mat (fix D2c), which includes
        votes on moderated-out comments. This matches Clojure, where
        zero-out-columns keeps moderated-out columns in the matrix (zeroed but
        present). Since raw_rating_mat contains all historical votes and votes
        are immutable in PostgreSQL, monotonicity is guaranteed without explicit
        persistence — a participant who once qualified can never lose votes.
        If the code is ever refactored to use delta vote processing, in-conv
        MUST be persisted to DynamoDB. See compdemocracy/polis#2358 and
        Clojure's approach in conv_man.clj:55, conversation.clj:244.

        Returns:
            Set of participant IDs that meet the threshold
        """
        n_cmts = len(self.raw_rating_mat.columns) if hasattr(self.raw_rating_mat, 'columns') else 0
        threshold = min(7, n_cmts)

        # Get vote counts for all participants
        vote_counts = self._compute_user_vote_counts()

        # Filter participants meeting threshold
        in_conv = {pid for pid, count in vote_counts.items() if count >= threshold}

delphi/tests/test_discrepancy_fixes.py:669

• test_two_prop_test_with_pseudocounts currently uses (succ1,n1)=(10,20) and (succ2,n2)=(15,30), which both produce p=0.5. That makes both the plain two-proportion z-test and the pseudocount-adjusted version return ~0, so the test will pass even if the pseudocount adjustment is not implemented. Use inputs where p1 != p2 (and ideally where the +1/+2 adjustment materially changes the z-score) so this test actually detects the D6 discrepancy.

    def test_two_prop_test_with_pseudocounts(self):
        """two_prop_test should add +1 pseudocounts matching Clojure."""
        # With pseudocounts: (succ+1)/(n+2) for both groups
        succ1, n1 = 10, 20
        succ2, n2 = 15, 30

        # Clojure formula adds +1 to successes and +2 to trials
        p1_clj = (succ1 + 1) / (n1 + 2)
        p2_clj = (succ2 + 1) / (n2 + 2)
        p_pooled_clj = (succ1 + succ2 + 2) / (n1 + n2 + 4)
        se_clj = math.sqrt(p_pooled_clj * (1 - p_pooled_clj) * (1 / (n1 + 2) + 1 / (n2 + 2)))
        expected = (p1_clj - p2_clj) / se_clj if se_clj > 0 else 0.0

        # Python currently doesn't add pseudocounts
        p1_py = succ1 / n1
        p2_py = succ2 / n2
        python_result = two_prop_test(p1_py, n1, p2_py, n2)

        print(f"two_prop_test: Python={python_result:.4f}, Clojure(with pseudocounts)={expected:.4f}")
        check.almost_equal(python_result, expected, abs=0.01,
                            msg=f"two_prop_test should include pseudocounts: Python={python_result:.4f}, expected={expected:.4f}")

delphi/tests/test_discrepancy_fixes.py:575

• The docstring says this asserts repness is non-empty "with correct thresholds", but the test doesn't validate the Z-score thresholds (and D9 is still tracked via the z90/z95 xfail tests). As written, this can pass even when thresholds are still wrong, which makes it misleading as a D9 regression signal. Consider either tying this assertion to the threshold constants (or to Clojure reference output), or moving it out of the D9 section / rewording to avoid implying it validates the thresholds.

    def test_repness_not_empty(self, conv, dataset_name):
        """Repness should produce non-empty comment_repness with correct thresholds."""
        repness = conv.repness
        check.is_not_none(repness, "Repness should not be None")
        if repness:
            check.is_in('comment_repness', repness, "Should have comment_repness key")
            if 'comment_repness' in repness:
                check.greater(len(repness['comment_repness']), 0,
                              "comment_repness should not be empty")

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[github-actions]

Delphi Coverage Report

File	Stmts	Miss	Cover
init.py	2	0	100%
benchmarks/bench_pca.py	76	76	0%
benchmarks/bench_repness.py	81	81	0%
benchmarks/bench_update_votes.py	38	38	0%
benchmarks/benchmark_utils.py	34	34	0%
components/init.py	1	0	100%
components/config.py	165	133	19%
conversation/init.py	2	0	100%
conversation/conversation.py	1117	328	71%
conversation/manager.py	131	42	68%
database/init.py	1	0	100%
database/dynamodb.py	387	234	40%
database/postgres.py	305	205	33%
pca_kmeans_rep/init.py	5	0	100%
pca_kmeans_rep/clusters.py	257	22	91%
pca_kmeans_rep/corr.py	98	17	83%
pca_kmeans_rep/pca.py	52	16	69%
pca_kmeans_rep/repness.py	361	47	87%
pca_kmeans_rep/stats.py	107	22	79%
regression/init.py	4	0	100%
regression/clojure_comparer.py	188	17	91%
regression/comparer.py	887	720	19%
regression/datasets.py	135	27	80%
regression/recorder.py	36	27	25%
regression/utils.py	137	118	14%
run_math_pipeline.py	260	114	56%
umap_narrative/500_generate_embedding_umap_cluster.py	210	109	48%
umap_narrative/501_calculate_comment_extremity.py	112	54	52%
umap_narrative/502_calculate_priorities.py	135	135	0%
umap_narrative/700_datamapplot_for_layer.py	502	502	0%
umap_narrative/701_static_datamapplot_for_layer.py	310	310	0%
umap_narrative/702_consensus_divisive_datamapplot.py	432	432	0%
umap_narrative/801_narrative_report_batch.py	785	785	0%
umap_narrative/802_process_batch_results.py	265	265	0%
umap_narrative/803_check_batch_status.py	175	175	0%
umap_narrative/llm_factory_constructor/init.py	2	2	0%
umap_narrative/llm_factory_constructor/model_provider.py	157	157	0%
umap_narrative/polismath_commentgraph/init.py	1	0	100%
umap_narrative/polismath_commentgraph/cli.py	270	270	0%
umap_narrative/polismath_commentgraph/core/init.py	3	3	0%
umap_narrative/polismath_commentgraph/core/clustering.py	108	108	0%
umap_narrative/polismath_commentgraph/core/embedding.py	104	104	0%
umap_narrative/polismath_commentgraph/lambda_handler.py	219	219	0%
umap_narrative/polismath_commentgraph/schemas/init.py	2	0	100%
umap_narrative/polismath_commentgraph/schemas/dynamo_models.py	160	9	94%
umap_narrative/polismath_commentgraph/tests/conftest.py	17	17	0%
umap_narrative/polismath_commentgraph/tests/test_clustering.py	74	74	0%
umap_narrative/polismath_commentgraph/tests/test_embedding.py	55	55	0%
umap_narrative/polismath_commentgraph/tests/test_storage.py	87	87	0%
umap_narrative/polismath_commentgraph/utils/init.py	3	0	100%
umap_narrative/polismath_commentgraph/utils/converter.py	283	237	16%
umap_narrative/polismath_commentgraph/utils/group_data.py	354	336	5%
umap_narrative/polismath_commentgraph/utils/storage.py	584	477	18%
umap_narrative/reset_conversation.py	159	50	69%
umap_narrative/run_pipeline.py	453	312	31%
utils/general.py	62	41	34%
Total	10950	7643	30%

[jucor]

Superseded by spr-managed PR stack. See the new stack starting at #2508.