diff --git a/delphi/docs/CLJ-PARITY-FIXES-JOURNAL.md b/delphi/docs/CLJ-PARITY-FIXES-JOURNAL.md index 629179770..6dedd4454 100644 --- a/delphi/docs/CLJ-PARITY-FIXES-JOURNAL.md +++ b/delphi/docs/CLJ-PARITY-FIXES-JOURNAL.md @@ -1216,3 +1216,293 @@ ever appears in real Polis data. It does. out of this committed doc; the unredacted findings are in Claude's per-project memory store (`~/.claude/projects/...`). Open a follow-up discussion with the team before any user-facing action. + +## Session: PR 14a — Scalar deletion (2026-06-11) + +Foundation pass before D10/D11/D12. The scalar implementations in +`repness.py` were test-only (production calls only `compute_group_comment_stats_df` ++ `select_rep_comments_df` + `select_consensus_comments_df` via +`conv_repness`). Deleting them removes the "where do I put this helper?" +ambiguity for D10/D11/D12 (which all add new helpers to `repness.py`) and +shrinks the test surface by ~35 obsolete unit tests. + +### What landed + +**Production code (`delphi/polismath/pca_kmeans_rep/repness.py`)** — 445 lines deleted: +- DELETE primitives: `prop_test`, `two_prop_test` (both also dead in production — + only test consumers). +- DELETE orchestration: `comment_stats`, `add_comparative_stats`, `repness_metric`, + `finalize_cmt_stats`, `passes_by_test`, `best_agree`, `best_disagree`, + `select_rep_comments`, `select_consensus_comments`. +- DELETE unused: `calculate_kl_divergence` (no callers anywhere). +- KEEP: `z_score_sig_90`, `z_score_sig_95` (trivial threshold checks used scalar-side + in selection logic; vectorizing them would not save lines). +- ENRICH docstrings: `prop_test_vectorized` and `two_prop_test_vectorized` now + embed the scalar-equivalent closed-form algebra (so the formula stays readable + even though the scalar functions are gone). Pattern from the user during the + PR 14a discussion: "where we cannot achieve readability on the vectorized, + put in comments showing the non-vectorized equivalent." + +**Tests** — net -35 passed tests (296 → 295... actually delta is 330 → 295): +- DELETE entirely: `tests/test_old_format_repness.py` (557 lines, mirror of + scalar-only tests in `test_repness_unit.py`; the "old format" was the scalar + dict-in/dict-out API). +- DELETE classes: `TestCommentStats`, `TestSelectionFunctions`, + `TestConsensusAndGroupRepness` in `test_repness_unit.py`. Also + `TestStatisticalFunctions::test_prop_test` and `test_two_prop_test`. +- MIGRATE D4/D5/D6 BlobInjection (`test_discrepancy_fixes.py:1602+`) from + per-(gid, tid) scalar loop calls to a single vectorized call on a DataFrame + built from the blob's `repness` entries. This pattern (1) tests the actual + production code path, (2) produces a `.to_string()` diagnostic that beats + hand-formatted f-strings, (3) drops loop overhead. +- MIGRATE `TestD5ProportionTest::test_prop_test_matches_clojure_formula`, + `TestD6TwoPropTest::test_two_prop_test_matches_clojure_formula` + edge cases + to single vectorized calls on N-row DataFrames. +- CONSOLIDATE `TestD8FinalizeStats`'s 7 scalar boundary tests into one + parametrized DataFrame test (`test_repful_classification_boundary`) that + exercises the production `np.where(rat > rdt, 'agree', 'disagree')` logic. + Boundary cases preserved: `rat < rdt`, `rat > rdt`, `rat == rdt` (non-zero, + zero), negative z-scores. +- MIGRATE `TestD7RepnessMetric::test_metric_formula_is_product` to hand-computed + reference values (`1.3*1.8*0.8*2.5 = 4.68` for agree, `0.7*-0.9*0.2*-1.5 = 0.189` + for disagree — signed product). +- DELETE redundant scalar-formula tests in `TestSyntheticEdgeCases` + (test_prop_test_matches_clojure_formula_synthetic — duplicated by migrated + TestD5ProportionTest; test_clojure_repness_metric_product — duplicated by + TestD7RepnessMetric; test_clojure_repful_uses_rat_vs_rdt — purely tautological). +- Rename misleading `test_compute_group_comment_stats_matches_scalar` → + `test_compute_group_comment_stats_consistency_with_conv_repness`. +- Cross-checks in `TestVectorizedFunctions` (test_repness_unit.py) replaced + inline scalar calls with `_prop_test_reference` / `_two_prop_test_reference` + closed-form staticmethods. + +### Suite delta (pre/post PR 14a) + +- Pre-baseline (edge @ 2dce7385f): **330 passed, 12 skipped, 58 xfailed**. +- Post (@ this PR): **295 passed, 12 skipped, 58 xfailed**. +- Delta: -35 passed, 0 failed, 0 new xfailed. The -35 matches the deleted + scalar-only test count (test_old_format_repness ~20 + scalar classes in + test_repness_unit ~9 + scalar test methods in test_discrepancy_fixes ~6 + consolidated/removed). + +### For PR 14c (readability refactor) + +PR 14c will refactor `compute_group_comment_stats_df` for readability and +needs to mirror the scalar recipe. **The deleted scalar code is the +reference.** Retrieve via: + +```bash +git show ~1:delphi/polismath/pca_kmeans_rep/repness.py \ + | sed -n '161,302p' +``` + +Specifically (using the pre-deletion line numbers — file was 1008 lines at +edge HEAD 2dce7385f): + +- `comment_stats` lines 161-201 — the per-(group, comment) recipe. +- `add_comparative_stats` lines 203-235 — in-vs-out comparison. +- `repness_metric` lines 237-271 — the `r*rt*p*pt` product. +- `finalize_cmt_stats` lines 273-301 — agree-vs-disagree branch. + +The pre-PR-14a commit hash will be the parent of PR 14a's commit. Clojure +originals: `math/src/polismath/math/repness.clj:78-100,173-188,191-200`. + +### Pyright noise (unrelated to PR 14a, raised same session) + +Discovered during PR 14a that pyright produces ~10 errors on `repness.py` from +pandas-stubs false positives (`pd.DataFrame(columns=...)`, `df['col'] = value`, +Series-vs-DataFrame narrowing). PR #2560 added a pyright config that points +at `delphi/.venv` but did NOT set any rule overrides, so default-mode +`reportArgumentType` / `reportIndexIssue` errors surface for valid pandas code. +Verified pre-existing on edge HEAD (not introduced by PR 14a). + +Handoff written: `~/polis/HANDOFF_PYRIGHT_PANDAS_STUBS.md`. Do NOT just turn +off rules globally — investigate pandas-stubs version, community patterns, +targeted ignores. Tracked as Claude task #8. + +### What's Next + +PR 14a unblocks (in stack order): +1. **D10** — Rep comment selection. Research agent already produced a fix + proposal (this session). Helpers `passes_by_test`, `beats_best_by_test`, + `beats_best_agr` go top-level in `repness.py`; reduce structure uses + `df.to_dict('records')` iteration with mutable `{sufficient, best, best_agree}` + state. Boundary cases identified for synthetic test fixtures. +2. **D11** — Consensus selection. Research agent produced a fix proposal: + needs new `consensus_stats_df(vote_matrix_df) -> pd.DataFrame` (whole-data, + not per-group), plus rewrite of `select_consensus_comments_df` with the + `{'agree': [...], 'disagree': [...]}` output shape (top 5 each). + `conv_repness` must grow `mod_out` kwarg. +3. **D12** — Comment priorities. Research agent produced a fix proposal: + Clojure source at `conversation.clj:311-330,341-352,648-679`; + `pca.clj:167-178`. Needs new `pca_project_cmnts`, `comment_extremity`, + `importance_metric`, `priority_metric` in Python. `meta_tids` shape mismatch + (Python set vs Clojure map) flagged. +4. **PR 14b** — Backfill missing blob injection tests (D7 metric, D8 finalize, + full stats-stage injection). +5. **Goldens** — Re-record `vw` and `biodiversity` (sklearn KMeans seeding + decision pending — see `delphi/scratch/COPILOT_MATH_QUESTIONS.md`). +6. **PR 14c** — Readability refactor of `compute_group_comment_stats_df`, + using the deleted scalar code (retrievable via `git show`) as the + readability reference. Research agent produced a clean split proposal: + `_build_group_comment_index` (plumbing) + `_compute_per_group_stats` + (math) + 5-line orchestrator. + +## Session: ns-PASS fix (2026-06-11) + +**Context.** While preparing the D10/D11/D12 rework on top of PR 14a, a +Clojure re-read surfaced a latent bug in `compute_group_comment_stats_df`: +`ns` (and `total_votes`) were computed as `na + nd`, silently dropping PASS +votes. Clojure's `:ns` is `(count-votes votes)` (math/repness.clj:56-61, +:70), which calls `(filter identity votes)`. In Clojure 0 is truthy, so +PASS (0) counts; only `nil` is filtered out. Therefore Clojure +`ns = na + nd + np`, and every downstream metric (`pa`, `pd`, `pat`, +`pdt`, `ra`, `rd`, `rat`, `rdt`, `agree_metric`, `disagree_metric`, plus +D11's `consensus_stats_df` which will mirror the same recipe at the +whole-conversation level) was off whenever PASS votes existed. + +**Why D5 BlobInjection didn't catch it.** D5's blob-injection tests pull +`(n-success, n-trials)` straight from the Clojure blob's `repness` +entries and feed them to `prop_test_vectorized`. They bypass +`compute_group_comment_stats_df` entirely, so the bug downstream of +`ns = na + nd` was invisible. The same gap will recur for D11 / D12 until +we ship pure-formula tests that build a tiny vote matrix and assert on the +counts. Lesson: blob-injection is necessary but not sufficient — every +formula whose inputs are themselves computed by Python needs at least one +pure-formula unit test that exercises the input-building code. + +**TDD cycle.** +- **BASELINE** — full suite at PR 14a parent: 295 passed, 12 skipped, + 58 xfailed. +- **RED** — added `TestNsIncludesPassVotes` in `tests/test_repness_unit.py` + with four pure-formula tests: single-comment mixed AGREE/DISAGREE/PASS, + all-PASS column, NaN-vs-PASS distinction, two-group `other_votes` + including out-group PASS. All four failed on the buggy code (4 fails). +- **GREEN** — in `polismath/pca_kmeans_rep/repness.py`: + - `total_counts` now computes `total_votes=('vote', 'size')` directly in + the groupby agg, instead of `total_agree + total_disagree`. + - `group_counts` now computes `ns=('vote', 'size')` directly, instead of + `na + nd`. + - `'size'` on the already-`dropna(subset=['vote'])`-filtered frame counts + exactly the non-NaN entries — including PASS (0). This is the + Clojure `(count (filter identity votes))` recipe verbatim. + - Both sites carry a comment citing repness.clj:56-61, :70 and the + truthy-0 reasoning. + - Docstring updated: `ns` now documented as `agree + disagree + PASS`. +- **FULL SUITE** — 299 passed, 12 skipped, 58 xfailed. Delta = +4 + (exactly the new ns-PASS tests). No existing pre-PR-14a or PR-14a test + broke. The pre-existing `TestVectorizedFunctions` fixtures use only + AGREE/DISAGREE/NaN (no PASS), so they were never sensitive to the bug. + +**Cascade to D11.** D11's plan introduces a `consensus_stats_df(vote_matrix_df)` +function computing whole-conversation stats (the `:mod-out` Clojure path). +That function will inevitably mirror the same `na + nd + np` recipe — so +the ns-PASS fix lands BEFORE D10 in the stack to keep D11's implementation +clean. D11 should follow the same pure-formula test pattern: build a vote +matrix with mixed PASS and assert `ns == count of non-NaN cells`. + +**Goldens.** Stays DEFERRED. Re-recording is gated on +sklearn-KMeans-seeding consensus (see scratch/COPILOT_MATH_QUESTIONS.md); +no values shift at the goldens commit until D10/D11/D12 land. + +**Stack position.** New commit inserted between PR 14a (#2564) and D10 +(#2566). D10, D11, D12, goldens rebased cleanly on top; 2-sided docs +conflicts in PLAN/JOURNAL at each downstream commit resolved manually +to merge both edits (downstream docs additions kept; ns-PASS row and +session entry preserved). + +## Session: PR 8 — D10 rep comment selection (2026-06-11) + +Landed in `/goal` mode (autonomous run targeting D10 + D11 + D12 + goldens +as a stacked PR series). Decisions made without inline user check are +documented in `~/polis/D10_D11_D12_GOLDENS_DECISIONS.md` for batch review. + +### What landed + +**Production code (`delphi/polismath/pca_kmeans_rep/repness.py`)** — added +3 top-level helpers + `_finalize_row_for_output` + rewrote `select_rep_comments_df`: + +- `passes_by_test(s) -> bool` — Clojure `passes-by-test?` (repness.clj:165-170). + OR'd on `(rat, pat)` and `(rdt, pdt)` z-sig-90. **NO `pa >= 0.5` gate** — + the pre-D10 Python gate was a botched-port over-restriction with no + Clojure analog. +- `beats_best_by_test(s, current_best_z) -> bool` — Clojure `beats-best-by-test?` + (repness.clj:133-139). Strict `>` on `max(rat, rdt)` vs current best z. +- `beats_best_agr(s, current_best) -> bool` — Clojure `beats-best-agr?` + (repness.clj:142-162). Four-branch agree-priority logic: + 1. `na == 0 and nd == 0` → reject. + 2. Current best AND `current_best['ra'] > 1.0` → compare 4-way signed + product `ra * rat * pa * pat`. + 3. Current best (else, `ra <= 1.0`) → compare `pa * pat`. + 4. No current best → accept if `z90(pat)` OR `(ra > 1.0 AND pa > 0.5)`. +- `_finalize_row_for_output(row, *, is_best_agree=False)` — Clojure + `finalize-cmt-stats` (repness.clj:173-188) + best-agree flagging + (repness.clj:262-264). Adds `best_agree=True` and `n_agree=na` keys for + the best-agree slot. +- `select_rep_comments_df(stats_df, mod_out=None) -> List[Dict[str, Any]]` — + single-pass reduce over `stats_df.to_dict('records')` mirroring Clojure + `select-rep-comments` (repness.clj:212-281). Per-row state + `{sufficient, best, best_agree}` updated by the three helpers; final + assembly is dedup-best-agree-from-sufficient → sort by metric → + prepend best-agree → take 5 → agrees-before-disagrees. + +**Caller (`conv_repness`)** — dropped the `_stats_row_to_dict` wrapping +step since `select_rep_comments_df` now returns finalized dicts directly. + +**Two pre-D10 bugs fixed alongside the rewrite** (research-agent flagged): +- `pa >= 0.5 / pd >= 0.5` over-gate in the passing filter — removed. No + Clojure analog; was dropping legitimate candidates. +- "Fill-from-other-category" + "first-row" fallback blocks — deleted. The + `:best` / `:best_agree` mechanism IS the Clojure fallback. + +**Tests** — 18 new tests (in `tests/test_discrepancy_fixes.py`): +- `TestD10PassesByTest` (4 tests): agree-side significant, disagree-side + significant, neither significant, no `pa >= 0.5` gate. +- `TestD10BeatsBestByTest` (3 tests): None-best, max-rat-rdt, strict `>`. +- `TestD10BeatsBestAgr` (6 tests): Branch 1 (na=nd=0), Branch 2 (ra>1), + Branch 3 (ra<=1), Branch 4 z90(pat), Branch 4 (ra>1 AND pa>0.5), Branch 4 + rejection. +- `TestD10SelectRepCommentsBoundary` (5 tests): empty input, single unvoted + row → best fallback, sufficient-empty-best-agree-only, take-5 cap with + agrees-before-disagrees ordering, **the eviction edge case** (best_agree + outside sufficient evicting 5th-highest-metric). + +**Re-xfailed with updated reasons** (D14 / D1 upstream divergence): +- `TestD9ZScoreThresholds::test_z_values_match_clojure` +- `TestD5ProportionTest::test_pat_values_match_clojure_blob` +- `TestD6TwoPropTest::test_rat_values_match_clojure_blob` +- `TestD7RepnessMetric::test_repness_metric_matches_clojure_blob` +- `TestD8FinalizeStats::test_repful_matches_clojure_blob` +- `TestD10RepCommentSelection::test_rep_comments_match_clojure` + +Why xfailed despite D10 landing: D10 enables shared comments in the +selection (overlap rises from 0% to ~20% on vw cold_start), but +per-(gid, tid) stats still mismatch because Python and Clojure put +different participants in the "same" group ID. That's upstream +PCA/KMeans group-membership divergence (D14 / D1), not D10. D10 is +verified via the 18 synthetic helper + boundary tests above. + +### Suite delta (pre/post D10) + +- Pre (post-14a): 295 passed, 12 skipped, 58 xfailed. +- Post (this PR): 313 passed, 12 skipped, 58 xfailed. +- Delta: +18 passed, 0 failed, 0 new xfailed. The +18 matches the 18 new + D10 synthetic tests exactly. + +### Decisions made autonomously (under `/goal` mode) + +See `~/polis/D10_D11_D12_GOLDENS_DECISIONS.md`. Highlights: +- **S1**: Python convention key names (`repful`, `best_agree`, `n_agree`) + instead of Clojure hyphens. Math blob alignment is a future PR. +- **S2**: `select_rep_comments_df` returns `List[Dict[str, Any]]` instead + of `pd.DataFrame` — variable extra keys (best_agree flag) make list-of- + dicts cleaner than DF-with-NaN-columns. +- **D10.1**: Two pre-D10 bugs (pa>=0.5 gate, fill-from-other fallback) + folded into D10 rather than separate PRs — the rewrite replaces the + function so a surgical fix would be more noise than value. +- **D10.7**: Real-data blob-comparison tests re-xfailed with reasons + pointing at D14/D1, not softened to overlap-thresholds — more honest. + +### What's Next + +PR 9 (D11) on top of D10 in the same spr stack. diff --git a/delphi/docs/PLAN_DISCREPANCY_FIXES.md b/delphi/docs/PLAN_DISCREPANCY_FIXES.md index d2df7b48d..479eb77e3 100644 --- a/delphi/docs/PLAN_DISCREPANCY_FIXES.md +++ b/delphi/docs/PLAN_DISCREPANCY_FIXES.md @@ -28,7 +28,9 @@ This plan's "PR N" labels map to actual GitHub PRs as follows: | PR 7 (D8) | #2522 | Stack 15/17 | Fix D8: finalize comment stats | | PR 12 (D15) | #2523 | Stack 16/17 | Fix D15: moderation handling | | (K-inv) | #2524 | Stack 17/17 | Fix K-means k divergence: preserve row order | -| PR 8 (D10) | — (WIP) | — | Fix D10: rep comment selection — **NEEDS REWORK** | +| PR 14a (scalar deletion) | #2564 | — | Delete dead scalar paths in `repness.py`; migrate blob injection tests to vectorized | +| PR ns-PASS fix | — (in flight) | — | Fix `ns` to include PASS votes in `compute_group_comment_stats_df` (Clojure `count-votes` parity) | +| PR 8 (D10) | — (in flight) | — | Fix D10: rep comment selection — single-pass reduce matching Clojure | | PR 9 (D11) | — (WIP) | — | Fix D11: consensus selection — **NEEDS REWORK** | | PR 10 (D3) | — (WIP) | — | Fix D3: k-smoother buffer — **NEEDS REWORK** | | PR 11 (D12) | — (WIP) | — | Fix D12: comment priorities — **NEEDS REWORK** | @@ -914,3 +916,14 @@ Tagging this as a follow-up. No code changes until we discuss. - **`to_dynamo_dict` parallel inline implementations** were refactored to route through the same helpers as `to_dict` in PR #2523 follow-up. No further action needed. +- **D10 take-5 eviction edge case** (2026-06-11). The Clojure-parity + `select_rep_comments_df` introduced in PR 8 mirrors Clojure exactly: + `take(5)` runs AFTER prepending the `best_agree` slot. When `best_agree` + was kept by `beats_best_agr?` as a non-significant agree-priority fallback + (i.e. it failed `passes_by_test?` but qualified via Branch 4) AND + `:sufficient` already has 5 entries, the prepend pushes the total to 6 + and `take(5)` silently evicts the 5th-highest-metric `sufficient` entry + — possibly a strong dissenting view. Mirrored for blob parity; flag for + future product review. See `# TODO(parity-eviction)` in + `delphi/polismath/pca_kmeans_rep/repness.py::select_rep_comments_df` and + the synthetic test `TestD10SelectRepCommentsBoundary::test_take_5_eviction_when_best_agree_outside_sufficient`. diff --git a/delphi/polismath/pca_kmeans_rep/repness.py b/delphi/polismath/pca_kmeans_rep/repness.py index bbc5fb1f4..ea522b5b9 100644 --- a/delphi/polismath/pca_kmeans_rep/repness.py +++ b/delphi/polismath/pca_kmeans_rep/repness.py @@ -7,10 +7,7 @@ import numpy as np import pandas as pd -from typing import Dict, List, Optional, Tuple, Union, Any -from copy import deepcopy -import math -from scipy import stats +from typing import Any, Dict, Iterable, List, Optional, Tuple from polismath.utils.general import AGREE, DISAGREE @@ -66,474 +63,44 @@ def z_score_sig_95(z: float) -> bool: return z > Z_95 -def prop_test(succ: int, n: int) -> float: - """ - One-proportion z-test, matching Clojure's stats/prop-test (stats.clj:10-15). - - Clojure formula: - (let [[succ n] (map inc [succ n])] - (* 2 (sqrt n) (+ (/ succ n) -0.5))) - - Which simplifies to: 2 * sqrt(n+1) * ((succ+1)/(n+1) - 0.5) - - This is a Wilson-score-like test with built-in +1 pseudocount (Laplace - smoothing). Unlike the standard z-test ((p - p0) / sqrt(p0*(1-p0)/n)), - the +1 terms regularize extreme values for small samples, preventing - spurious significance in small Polis groups. - - Note: the pseudocount here (+1 to succ and n, i.e. Beta(1,1)) is - independent of the PSEUDO_COUNT used for pa/pd computation (Beta(2,2)). - Clojure's prop-test takes raw success counts, not pre-smoothed - probabilities. - - Args: - succ: Number of successes (e.g. agrees `na` or disagrees `nd`) - n: Number of *trials counted as votes for this test*. In all current - callers, this is `ns = na + nd` (AGREE + DISAGREE) — PASS votes - are NOT included, matching what Clojure passes as `n-trials`. This - is a Polis-pipeline convention, not a generic z-test signature; - if you call this from elsewhere, supply `na + nd` rather than - a "total votes seen including pass" count. - - Returns: - Z-score. Positive when the smoothed proportion (succ+1)/(n+1) > 0.5 - (equivalent to succ >= n/2). This differs slightly from the raw-ratio - condition succ/n > 0.5 because of the +1 pseudocount applied to both - numerator and denominator. - - No n=0 short-circuit (Clojure parity — stats.clj:10-15 has no guard): - prop_test(0, 0) → (1, 1) after +1 → 2*sqrt(1)*(1/1 - 0.5) = 1.0. - """ - # Apply +1 pseudocount to both numerator and denominator - succ_pc = succ + 1 - n_pc = n + 1 - return 2 * math.sqrt(n_pc) * (succ_pc / n_pc - 0.5) - - -def two_prop_test(succ_in: int, succ_out: int, pop_in: int, pop_out: int) -> float: - """ - Two-proportion z-test with +1 pseudocount on all inputs. - - Matches Clojure's stats/two-prop-test (stats.clj:18-33): - (let [[succ-in succ-out pop-in pop-out] (map inc [succ-in succ-out pop-in pop-out]) - pi1 (/ succ-in pop-in) - pi2 (/ succ-out pop-out) - pi-hat (/ (+ succ-in succ-out) (+ pop-in pop-out))] - ...) - - The +1 pseudocount (Laplace smoothing) regularizes the z-score for small - samples, preventing extreme values when group sizes are tiny. - - Args: - succ_in: Number of successes in the group (e.g., agrees) - succ_out: Number of successes outside the group - pop_in: Total votes in the group - pop_out: Total votes outside the group - - Returns: - Z-score (positive means group proportion > other proportion) - """ - # No pop_in/pop_out short-circuit: Clojure's (map inc ...) increments all - # four inputs unconditionally, so pop=0 becomes pop=1 and the test proceeds. - # The only early-return is pi_hat == 1 below, matching Clojure. - - # Add +1 pseudocount to all four inputs (Clojure: map inc) - s1 = succ_in + 1 - s2 = succ_out + 1 - p1 = pop_in + 1 - p2 = pop_out + 1 - - pi1 = s1 / p1 - pi2 = s2 / p2 - pi_hat = (s1 + s2) / (p1 + p2) - - if pi_hat == 1.0: - # Clojure note (stats.clj:26-27): "this isn't quite right... could - # actually solve this using limits" — returning 0 for now, matching Clojure. - return 0.0 - - se = math.sqrt(pi_hat * (1 - pi_hat) * (1/p1 + 1/p2)) - if se == 0: - return 0.0 - return (pi1 - pi2) / se - - -def comment_stats(votes: np.ndarray, group_members: List[int]) -> Dict[str, Any]: - """ - Calculate basic stats for a comment within a group. - - Args: - votes: Array of votes (-1, 0, 1, or None) for the comment - group_members: Indices of group members - - Returns: - Dictionary of statistics - """ - # Filter votes to only include group members - group_votes = votes[group_members] - - # Count agrees, disagrees, and total votes - n_agree = np.sum(group_votes == AGREE) - n_disagree = np.sum(group_votes == DISAGREE) - n_votes = n_agree + n_disagree - - # Calculate probabilities with pseudocounts (Bayesian smoothing) - p_agree = (n_agree + PSEUDO_COUNT/2) / (n_votes + PSEUDO_COUNT) if n_votes > 0 else 0.5 - p_disagree = (n_disagree + PSEUDO_COUNT/2) / (n_votes + PSEUDO_COUNT) if n_votes > 0 else 0.5 - - # Calculate significance tests — pass raw counts, matching Clojure's - # (stats/prop-test na ns) and (stats/prop-test nd ns) (repness.clj:74-75) - # No n_votes>0 guard — Clojure parity (stats.clj:10-15 has no n=0 short-circuit; - # prop_test handles n=0 via the +1 pseudocount → returns 1.0) - p_agree_test = prop_test(n_agree, n_votes) - p_disagree_test = prop_test(n_disagree, n_votes) - - # Return stats - return { - 'na': n_agree, - 'nd': n_disagree, - 'ns': n_votes, - 'pa': p_agree, - 'pd': p_disagree, - 'pat': p_agree_test, - 'pdt': p_disagree_test - } - - -def add_comparative_stats(comment_stats: Dict[str, Any], - other_stats: Dict[str, Any]) -> Dict[str, Any]: - """ - Add comparative statistics between a group and others. - - Args: - comment_stats: Statistics for the group - other_stats: Statistics for other groups combined - - Returns: - Enhanced statistics with comparative measures - """ - result = deepcopy(comment_stats) - - # Calculate representativeness ratios - result['ra'] = result['pa'] / other_stats['pa'] if other_stats['pa'] > 0 else 1.0 - result['rd'] = result['pd'] / other_stats['pd'] if other_stats['pd'] > 0 else 1.0 - - # Calculate representativeness tests — pass raw counts, matching Clojure's - # (stats/two-prop-test (:na in-stats) (sum :na rest-stats) - # (:ns in-stats) (sum :ns rest-stats)) (repness.clj:97-100) - result['rat'] = two_prop_test( - result['na'], other_stats['na'], - result['ns'], other_stats['ns'] - ) - - result['rdt'] = two_prop_test( - result['nd'], other_stats['nd'], - result['ns'], other_stats['ns'] - ) - - return result - - -def repness_metric(stats: Dict[str, Any], key_prefix: str) -> float: - """ - Composite representativeness score, matching Clojure's repness-metric. - - Clojure (math/src/polismath/math/repness.clj:191-193): - (defn repness-metric - [{:keys [repness repness-test p-success p-test]}] - (* repness repness-test p-success p-test)) - - For Python the keys are looked up via key_prefix: - 'a' (agree) → ra * rat * pa * pat - 'd' (disagree) → rd * rdt * pd * pdt - - This is a *signed* product of 4 values — there is no abs(). A negative - z-score (pat / rat / pdt / rdt) flips the sign of the metric, exactly as - in Clojure. Downstream `select_rep_comments` sorts candidates by this - metric in descending order and keeps the top N, so negative metrics rank - at the bottom of the candidate pool. They are not actively *filtered* - here, though — fallback paths (e.g. fewer than the requested N candidates - pass significance) can still surface a negative-metric comment. Callers - that need strict positive-metric semantics should gate at the call site. - - Args: - stats: Statistics for a comment/group - key_prefix: 'a' for agreement, 'd' for disagreement - - Returns: - Composite representativeness score (signed product of 4 values). - """ - p = stats[f'p{key_prefix}'] - p_test = stats[f'p{key_prefix}t'] - r = stats[f'r{key_prefix}'] - r_test = stats[f'r{key_prefix}t'] - return r * r_test * p * p_test - - -def finalize_cmt_stats(stats: Dict[str, Any]) -> Dict[str, Any]: - """ - Finalize comment stats and classify as agree/disagree, matching Clojure. - - Clojure (math/src/polismath/math/repness.clj:173-180): - (defn finalize-cmt-stats - [tid {:keys [... rat rdt ...]}] - (let [[...] (if (> rat rdt) - [na ns pa pat ra rat :agree] - [nd ns pd pdt rd rdt :disagree])] - ...)) - - Pure comparison of the two two-prop z-scores. No probability/ratio - threshold logic — strict `rat > rdt` (rat == rdt falls through to disagree). - Always populates `agree_metric` / `disagree_metric` (used downstream by - selection routines that rank candidates). - - Args: - stats: Statistics for a comment/group - - Returns: - Finalized statistics with `repful`, `agree_metric`, `disagree_metric`. - """ - result = deepcopy(stats) - result['agree_metric'] = repness_metric(stats, 'a') - result['disagree_metric'] = repness_metric(stats, 'd') - result['repful'] = 'agree' if stats['rat'] > stats['rdt'] else 'disagree' - return result - - -def passes_by_test(stats: Dict[str, Any], repful: str, p_thresh: float = 0.5) -> bool: - """ - Check if comment passes significance tests. - - Args: - stats: Statistics for a comment/group - repful: 'agree' or 'disagree' - p_thresh: Probability threshold - - Returns: - True if passes significance tests - """ - key_prefix = 'a' if repful == 'agree' else 'd' - p = stats[f'p{key_prefix}'] - p_test = stats[f'p{key_prefix}t'] - r_test = stats[f'r{key_prefix}t'] - - # Check if proportion is high enough - if p < p_thresh: - return False - - # Check significance tests - return z_score_sig_90(p_test) and z_score_sig_90(r_test) - - -def best_agree(all_stats: List[Dict[str, Any]]) -> List[Dict[str, Any]]: - """ - Filter for best agreement comments. - - Args: - all_stats: List of comment statistics - - Returns: - Filtered list of comments that are best representatives by agreement - """ - # Filter to comments more agreed with than disagreed with - agree_stats = [s for s in all_stats if s['pa'] > s['pd']] - - # Filter to comments that pass significance tests - passing = [s for s in agree_stats if passes_by_test(s, 'agree')] - - if passing: - return passing - else: - return agree_stats - - -def best_disagree(all_stats: List[Dict[str, Any]]) -> List[Dict[str, Any]]: - """ - Filter for best disagreement comments. - - Args: - all_stats: List of comment statistics - - Returns: - Filtered list of comments that are best representatives by disagreement - """ - # Filter to comments more disagreed with than agreed with - disagree_stats = [s for s in all_stats if s['pd'] > s['pa']] - - # Filter to comments that pass significance tests - passing = [s for s in disagree_stats if passes_by_test(s, 'disagree')] - - if passing: - return passing - else: - return disagree_stats - - -def select_rep_comments(all_stats: List[Dict[str, Any]], - agree_count: int = 3, - disagree_count: int = 2) -> List[Dict[str, Any]]: - """ - Select representative comments for a group. - - Args: - all_stats: List of comment statistics - agree_count: Number of agreement comments to select - disagree_count: Number of disagreement comments to select - - Returns: - List of selected representative comments - """ - if not all_stats: - return [] - - # Start with best agreement comments - agree_comments = best_agree(all_stats) - - # Sort by agreement metric - agree_comments = sorted( - agree_comments, - key=lambda s: s['agree_metric'], - reverse=True - ) - - # Start with best disagreement comments - disagree_comments = best_disagree(all_stats) - - # Sort by disagreement metric - disagree_comments = sorted( - disagree_comments, - key=lambda s: s['disagree_metric'], - reverse=True - ) - - # Select top comments - selected = [] - - # Add agreement comments - for i, cmt in enumerate(agree_comments): - if i < agree_count: - cmt_copy = deepcopy(cmt) - cmt_copy['repful'] = 'agree' - selected.append(cmt_copy) - - # Add disagreement comments - for i, cmt in enumerate(disagree_comments): - if i < disagree_count: - cmt_copy = deepcopy(cmt) - cmt_copy['repful'] = 'disagree' - selected.append(cmt_copy) - - # If we couldn't find enough, try to add more from the other category - if len(selected) < agree_count + disagree_count: - # Add more agreement comments if needed - if len(selected) < agree_count + disagree_count and len(agree_comments) > agree_count: - for i in range(agree_count, min(len(agree_comments), agree_count + disagree_count)): - cmt_copy = deepcopy(agree_comments[i]) - cmt_copy['repful'] = 'agree' - selected.append(cmt_copy) - - # Add more disagreement comments if needed - if len(selected) < agree_count + disagree_count and len(disagree_comments) > disagree_count: - for i in range(disagree_count, min(len(disagree_comments), agree_count + disagree_count)): - cmt_copy = deepcopy(disagree_comments[i]) - cmt_copy['repful'] = 'disagree' - selected.append(cmt_copy) - - # If still not enough, at least ensure one comment - if not selected and all_stats: - # Just take the first one - cmt_copy = deepcopy(all_stats[0]) - cmt_copy['repful'] = cmt_copy.get('repful', 'agree') - selected.append(cmt_copy) - - return selected - - -def calculate_kl_divergence(p: np.ndarray, q: np.ndarray) -> float: - """ - Calculate Kullback-Leibler divergence between two probability distributions. - - Args: - p: First probability distribution - q: Second probability distribution - - Returns: - KL divergence - """ - # Replace zeros to avoid division by zero - p = np.where(p == 0, 1e-10, p) - q = np.where(q == 0, 1e-10, q) - - # numpy stubs: np.where widens p to ndarray|bool_, so np.sum is typed bool_. - # See pyright #2811. - return np.sum(p * np.log(p / q)) # pyright: ignore[reportReturnType] - - -def select_consensus_comments(all_stats: List[Dict[str, Any]]) -> List[Dict[str, Any]]: - """ - Select comments with broad consensus. - - Args: - all_stats: List of comment statistics for all groups - - Returns: - List of consensus comments - """ - # Group by comment - by_comment = {} - for stat in all_stats: - cid = stat['comment_id'] - if cid not in by_comment: - by_comment[cid] = [] - by_comment[cid].append(stat) - - # Comments that have stats for all groups - consensus_candidates = [] - - for cid, stats in by_comment.items(): - # Check if all groups mostly agree - all_agree = all(s['pa'] > 0.6 for s in stats) - - if all_agree: - # Calculate average agreement - avg_agree = sum(s['pa'] for s in stats) / len(stats) - - # Add as consensus candidate - consensus_candidates.append({ - 'comment_id': cid, - 'avg_agree': avg_agree, - 'repful': 'consensus', - 'stats': stats - }) - - # Sort by average agreement - consensus_candidates.sort(key=lambda x: x['avg_agree'], reverse=True) - - # Take top 2 - return consensus_candidates[:2] - - # ============================================================================= # Vectorized DataFrame-native functions for multi-group operations # ============================================================================= def prop_test_vectorized(succ: pd.Series, n: pd.Series) -> pd.Series: """ - Vectorized one-proportion z-test, matching Clojure's stats/prop-test. + Vectorized one-proportion z-test, matching Clojure's stats/prop-test + (math/src/polismath/math/stats.clj:10-15). - Formula: 2 * sqrt(n+1) * ((succ+1)/(n+1) - 0.5) + Scalar equivalent (the formula this implements element-wise): - See prop_test() docstring for derivation and rationale. + def prop_test(succ, n): + return 2 * sqrt(n + 1) * ((succ + 1) / (n + 1) - 0.5) + + Wilson-score-like test with built-in +1 pseudocount (Laplace / Beta(1,1) + smoothing). The +1 terms regularize extreme values for small samples, + preventing spurious significance in small Polis groups. Unlike the standard + z-test ((p - p0) / sqrt(p0*(1-p0)/n)), this formulation never divides by + zero — n=0 collapses to `2*sqrt(1)*(1/1 - 0.5) = 1.0` after smoothing. + + No n=0 short-circuit (Clojure parity — stats.clj:10-15 has no guard). + + Note: the pseudocount here (Beta(1,1)) is independent of the PSEUDO_COUNT + used for pa/pd computation (Beta(2,2)). prop_test takes RAW success and + trial counts, not pre-smoothed probabilities. Args: - succ: Series of success counts (e.g. `na` or `nd` per row) + succ: Series of success counts (e.g. `na` or `nd` per row). n: Series of trial counts. In all current callers this is `ns = na + nd` (AGREE + DISAGREE per row) — PASS votes are NOT included, matching - what Clojure passes as `n-trials`. See scalar `prop_test()` for the - same convention. + what Clojure passes as `n-trials`. If you call this from elsewhere, + supply `na + nd` rather than a "total votes seen including pass" count. Returns: - Series of z-scores + Series of z-scores. Positive when the smoothed proportion (succ+1)/(n+1) + > 0.5 (equivalent to succ >= n/2). Differs slightly from raw-ratio + succ/n > 0.5 because of the +1 pseudocount on both numerator and + denominator. """ succ_pc = succ + 1 n_pc = n + 1 @@ -549,19 +116,40 @@ def prop_test_vectorized(succ: pd.Series, n: pd.Series) -> pd.Series: def two_prop_test_vectorized(succ_in: pd.Series, succ_out: pd.Series, pop_in: pd.Series, pop_out: pd.Series) -> pd.Series: """ - Vectorized two-proportion z-test with +1 pseudocount on all inputs. + Vectorized two-proportion z-test with +1 pseudocount on all inputs, + matching Clojure's stats/two-prop-test + (math/src/polismath/math/stats.clj:18-33). + + Scalar equivalent (the formula this implements element-wise): - Matches Clojure's stats/two-prop-test (stats.clj:18-33). - See two_prop_test() scalar version for formula details. + def two_prop_test(succ_in, succ_out, pop_in, pop_out): + s1, s2 = succ_in + 1, succ_out + 1 + p1, p2 = pop_in + 1, pop_out + 1 + pi1, pi2 = s1 / p1, s2 / p2 + pi_hat = (s1 + s2) / (p1 + p2) + if pi_hat == 1.0: + return 0.0 # Clojure: "could solve via limits" (stats.clj:26-27) + se = sqrt(pi_hat * (1 - pi_hat) * (1/p1 + 1/p2)) + return (pi1 - pi2) / se + + +1 pseudocount (Laplace / Beta(1,1)) regularizes z-scores for small samples; + Clojure increments all four inputs unconditionally via (map inc ...) so + pop=0 becomes pop=1 and the test proceeds. The only early-return is + pi_hat == 1. + + No pop_in/pop_out short-circuit (Clojure parity). Vectorized handling: + - pi_hat == 1 → SE = 0 → z = NaN → fillna(0.0). + - pi_hat > 1 (na > pop, unreachable in real data) → sqrt of negative → NaN → 0.0. + - Division by zero → ±inf → replaced with 0.0. Args: - succ_in: Series of success counts in the group - succ_out: Series of success counts outside the group - pop_in: Series of total vote counts in the group - pop_out: Series of total vote counts outside the group + succ_in: Series of success counts in the group (e.g. agrees). + succ_out: Series of success counts outside the group. + pop_in: Series of total vote counts in the group. + pop_out: Series of total vote counts outside the group. Returns: - Series of z-scores + Series of z-scores (positive means group proportion > other proportion). """ # Add +1 pseudocount to all four inputs (Clojure: map inc) s1 = succ_in + 1 @@ -589,8 +177,9 @@ def compute_group_comment_stats_df(votes_long: pd.DataFrame, """ Compute vote counts and probabilities for all (group, comment) pairs. - This is the vectorized version of comment_stats() that operates on all - groups and comments simultaneously. + Vectorized port of Clojure's per-(group, comment) `comment-stats` recipe + (math/src/polismath/math/repness.clj:64-100). Operates on all groups and + comments simultaneously. Args: votes_long: Long-format DataFrame with columns: @@ -603,7 +192,8 @@ def compute_group_comment_stats_df(votes_long: pd.DataFrame, DataFrame indexed by (group_id, comment) with columns: - na: number of agrees - nd: number of disagrees - - ns: number of votes (agrees + disagrees) + - ns: number of votes (agrees + disagrees + PASS, Clojure parity; + see repness.clj:56-61, :70) - pa: probability of agree (with pseudocount smoothing) - pd: probability of disagree (with pseudocount smoothing) - pat: proportion test z-score for agree @@ -632,11 +222,17 @@ def compute_group_comment_stats_df(votes_long: pd.DataFrame, # Compute total counts per comment BEFORE filtering to group members # This matches the old behavior where "other" included ALL participants # not in the current group (even those not in any cluster) + # + # total_votes counts agree + disagree + PASS, matching Clojure's + # `count-votes` (math/src/polismath/math/repness.clj:56-61, :70). + # `count-votes` called with no `vote` arg uses `identity` as the filter + # predicate; in Clojure 0 is truthy, so PASS (0) votes are kept. NaN + # entries are already dropped above. Use size() to count non-NaN rows. total_counts = votes_only.groupby('comment').agg( total_agree=('vote', lambda x: (x == AGREE).sum()), total_disagree=('vote', lambda x: (x == DISAGREE).sum()), + total_votes=('vote', 'size'), ) - total_counts['total_votes'] = total_counts['total_agree'] + total_counts['total_disagree'] # Now add group column and filter to only group members votes_with_group = votes_only.copy() @@ -655,12 +251,18 @@ def compute_group_comment_stats_df(votes_long: pd.DataFrame, # Get all group IDs all_group_ids = [group['id'] for group in group_clusters] - # Compute vote counts per (group, comment) for votes from group members + # Compute vote counts per (group, comment) for votes from group members. + # + # ns counts agree + disagree + PASS, matching Clojure's `count-votes` + # (math/src/polismath/math/repness.clj:56-61, :70). `count-votes` with + # no `vote` arg uses `identity` as filter; in Clojure 0 is truthy, so + # PASS (0) votes count. NaN entries were already dropped above. Use + # size() to count non-NaN rows. group_counts = votes_in_groups.groupby(['group_id', 'comment']).agg( na=('vote', lambda x: (x == AGREE).sum()), nd=('vote', lambda x: (x == DISAGREE).sum()), + ns=('vote', 'size'), ) - group_counts['ns'] = group_counts['na'] + group_counts['nd'] # Create full index with all (group, comment) combinations to match old behavior # Old implementation: for each group, iterate over ALL comments (that have any votes) @@ -726,7 +328,10 @@ def compute_group_comment_stats_df(votes_long: pd.DataFrame, # Compute metrics # Clojure (repness.clj:191-193): (* repness repness-test p-success p-test) # agree_metric = ra * rat * pa * pat - # disagree_metric = rd * rdt * pd * pdt (signed product — see scalar repness_metric) + # disagree_metric = rd * rdt * pd * pdt + # Signed product — no abs(). Negative z-scores flip the sign of the metric; + # downstream selection sorts descending, so negative-metric comments rank + # at the bottom of the candidate pool but are not filtered here. stats_df['agree_metric'] = (stats_df['ra'] * stats_df['rat'] * stats_df['pa'] * stats_df['pat']) stats_df['disagree_metric'] = (stats_df['rd'] * stats_df['rdt'] @@ -739,102 +344,289 @@ def compute_group_comment_stats_df(votes_long: pd.DataFrame, return stats_df -def select_rep_comments_df(stats_df: pd.DataFrame, - agree_count: int = 3, - disagree_count: int = 2) -> pd.DataFrame: +# ============================================================================= +# D10: Selection helpers (Clojure parity for select-rep-comments) +# ============================================================================= +# +# Ports of Clojure's `select-rep-comments` and its three predicates from +# math/src/polismath/math/repness.clj:133-281. Operate on per-(group, comment) +# dict rows produced by `compute_group_comment_stats_df` (via to_dict('records')). +# Per-row dict ops + small per-group iteration (rather than vectorized) because +# `beats_best_agr` has a 4-branch decision against a moving "current best" +# that updates during iteration; vectorizing would require multiple passes +# without saving lines (per-group N typically <500 comments). + + +def passes_by_test(s: Dict[str, Any]) -> bool: + """ + Clojure passes-by-test? (repness.clj:165-170). + + True iff the agree side OR the disagree side passes z-sig-90 on BOTH + the proportion test (pat/pdt) and the representativeness test (rat/rdt). + No probability gate — pre-D10 Python's `pa >= 0.5` gate was a Python-only + over-restriction without a Clojure analog. + + (or (and (z-sig-90? rat) (z-sig-90? pat)) + (and (z-sig-90? rdt) (z-sig-90? pdt))) + """ + return ( + (z_score_sig_90(s['rat']) and z_score_sig_90(s['pat'])) + or (z_score_sig_90(s['rdt']) and z_score_sig_90(s['pdt'])) + ) + + +def beats_best_by_test(s: Dict[str, Any], current_best_z: Optional[float]) -> bool: + """ + Clojure beats-best-by-test? (repness.clj:133-139). + + True if `s` has a more-representative max(rat, rdt) than `current_best_z`, + OR if there is no current best yet. Strict `>` (Clojure: `>`). + + (or (nil? current-best-z) + (> (max rat rdt) current-best-z)) + """ + if current_best_z is None: + return True + return max(s['rat'], s['rdt']) > current_best_z + + +def beats_best_agr(s: Dict[str, Any], + current_best: Optional[Dict[str, Any]]) -> bool: + """ + Clojure beats-best-agr? (repness.clj:142-162). + + Four mutually exclusive branches: + + 1. `na == 0 and nd == 0`: reject. Comments with no votes never enter the + best-agree slot (Clojure: `(= 0 na nd)` → false). + 2. `current_best` exists AND `current_best['ra'] > 1.0`: compare the + 4-way signed product `ra * rat * pa * pat`. New row must beat the + current best on this product. + 3. `current_best` exists (else, i.e. `current_best['ra'] <= 1.0`): + compare `pa * pat` only — "shoot for something generally agreed upon" + when the current best isn't representative enough. + 4. No `current_best`: accept if `z90(pat)` OR `(ra > 1.0 AND pa > 0.5)`. + + `current_best` here is the RAW stats row (Clojure stores raw at + repness.clj:250 so this comparator keeps the `ra/rat/pa/pat` surface). """ - Select representative comments for a single group from a DataFrame. + if s['na'] == 0 and s['nd'] == 0: # Branch 1. + return False + if current_best is not None and current_best['ra'] > 1.0: # Branch 2. + return (s['ra'] * s['rat'] * s['pa'] * s['pat']) > ( + current_best['ra'] * current_best['rat'] + * current_best['pa'] * current_best['pat'] + ) + if current_best is not None: # Branch 3. + return (s['pa'] * s['pat']) > (current_best['pa'] * current_best['pat']) + # Branch 4. + return z_score_sig_90(s['pat']) or (s['ra'] > 1.0 and s['pa'] > 0.5) + + +def _finalize_row_for_output(row: Dict[str, Any], *, + is_best_agree: bool = False) -> Dict[str, Any]: + """ + Format a per-(group, comment) stats row for the final repness output + (math blob `repness` / `group_repness`). + + Mirrors Clojure `finalize-cmt-stats` (repness.clj:173-188) plus the + best-agree flagging at repness.clj:262-264. + + When `is_best_agree=True`, two extra keys are added: + - `best_agree`: True + - `n_agree`: the raw `na` (preserves the agree count even when the + row is classified as 'disagree' by `rat > rdt`). + + Key naming uses Python convention (underscored). Clojure-style hyphens + (`repful-for`, `n-agree`, etc.) are deferred to a future math-blob + alignment PR (see PLAN.md "Pending — needs team discussion"). + + `agree_metric` / `disagree_metric` are read directly from the row + (produced by `compute_group_comment_stats_df`) rather than recomputed. + Recomputing here would duplicate the formula at repness.clj:191-193 in + two places and risk drift if it ever changes (decision D10.8.3). + """ + repful = 'agree' if row['rat'] > row['rdt'] else 'disagree' + finalized: Dict[str, Any] = { + 'comment_id': row['comment'], + 'group_id': row['group_id'], + 'na': int(row['na']), + 'nd': int(row['nd']), + 'ns': int(row['ns']), + 'pa': row['pa'], + 'pd': row['pd'], + 'pat': row['pat'], + 'pdt': row['pdt'], + 'ra': row['ra'], + 'rd': row['rd'], + 'rat': row['rat'], + 'rdt': row['rdt'], + 'agree_metric': row['agree_metric'], + 'disagree_metric': row['disagree_metric'], + 'repful': repful, + } + if is_best_agree: + finalized['best_agree'] = True + finalized['n_agree'] = int(row['na']) + return finalized + - DataFrame-native version of select_rep_comments(). +def select_rep_comments_df(stats_df: pd.DataFrame, + mod_out: Optional[Iterable[int]] = None + ) -> Tuple[pd.DataFrame, Optional[Dict[str, Any]]]: + """ + Select representative comments for a single group (Clojure parity). + + Single-pass reduce over the group's (gid, tid) rows, mirroring + `select-rep-comments` in math/src/polismath/math/repness.clj:212-281. + + Per-row state {sufficient, best, best_agree}: + - `passes_by_test(row)` → append finalized row to `sufficient`. + - `:sufficient` still empty AND `beats_best_by_test` → update `best`. + - `beats_best_agr(row, best_agree)` → store RAW row as new `best_agree`. + + Final assembly (decision S2 / D10.4): + - `sufficient` non-empty: dedup best_agree from sufficient → sort by + agree/disagree metric (descending, signed product per repness.clj:191) + → take up to 5 (post-prepend → 4 sufficient max) → agrees-before- + disagrees on the sufficient slice. The best-agree dict is returned + SEPARATELY so the DataFrame stays clean (no NaN best_agree/n_agree + columns when the slot is empty). + - Else: `(empty_df, best_agree_dict)` if best_agree exists, else + `(single_row_df_for_best, None)` if best exists, else + `(empty_df, None)`. Args: - stats_df: DataFrame with comment statistics for ONE group - agree_count: Number of agreement comments to select - disagree_count: Number of disagreement comments to select + stats_df: DataFrame with comment statistics for ONE group, schema + as produced by `compute_group_comment_stats_df`. + mod_out: Optional iterable of tids to exclude (moderated-out comments). + Filter applied before the reduce (Clojure repness.clj:222). Returns: - DataFrame of selected representative comments - """ + `(rep_df, best_agree_dict)` tuple: + - `rep_df`: DataFrame of finalized rep-comment rows in math-blob + shape (see `_finalize_row_for_output`) — does NOT include the + best-agree slot, and carries NO `best_agree`/`n_agree` columns. + Already ordered agrees-before-disagrees and capped so that + `len(rep_df) + (1 if best_agree_dict else 0) <= 5`. + - `best_agree_dict`: Standalone finalized dict for the best-agree + slot (with `best_agree=True` and `n_agree=`), or `None` if + no candidate qualified. The caller is responsible for prepending + it to the flat output list. + """ + empty_df: pd.DataFrame = pd.DataFrame() + if stats_df.empty: - return stats_df - - total_wanted = agree_count + disagree_count - - # Best agree: pa > pd and passes significance tests - agree_candidates = stats_df[stats_df['pa'] > stats_df['pd']].copy() - if not agree_candidates.empty: - # Check significance: pat > Z_90 and rat > Z_90 - passing_agree = agree_candidates[ - (agree_candidates['pat'] > Z_90) & - (agree_candidates['rat'] > Z_90) & - (agree_candidates['pa'] >= 0.5) - ] - if not passing_agree.empty: - agree_candidates = passing_agree - - # Best disagree: pd > pa and passes significance tests - disagree_candidates = stats_df[stats_df['pd'] > stats_df['pa']].copy() - if not disagree_candidates.empty: - passing_disagree = disagree_candidates[ - (disagree_candidates['pdt'] > Z_90) & - (disagree_candidates['rdt'] > Z_90) & - (disagree_candidates['pd'] >= 0.5) - ] - if not passing_disagree.empty: - disagree_candidates = passing_disagree - - # Sort candidates by metric - if not agree_candidates.empty: - agree_candidates = agree_candidates.sort_values('agree_metric', ascending=False) - if not disagree_candidates.empty: - disagree_candidates = disagree_candidates.sort_values('disagree_metric', ascending=False) - - # Select top N from each category - selected_parts = [] - - if not agree_candidates.empty: - top_agree = agree_candidates.head(agree_count).copy() - top_agree['repful'] = 'agree' - selected_parts.append(top_agree) - - if not disagree_candidates.empty: - top_disagree = disagree_candidates.head(disagree_count).copy() - top_disagree['repful'] = 'disagree' - selected_parts.append(top_disagree) - - if selected_parts: - selected = pd.concat(selected_parts, ignore_index=False) - else: - selected = pd.DataFrame() - - # If we couldn't find enough, try to fill from available candidates - # This matches the exact behavior of the old select_rep_comments() function: - # - First fallback adds agree_comments[agree_count:min(len, total_wanted)] regardless of - # whether we exceed total_wanted (up to disagree_count more agrees) - # - Second fallback only runs if STILL < total_wanted - if len(selected) < total_wanted: - # Try to add more agree comments - # Old code: range(agree_count, min(len(agree_comments), agree_count + disagree_count)) - if not agree_candidates.empty and len(agree_candidates) > agree_count: - extra_limit = min(len(agree_candidates), total_wanted) - extra_agrees = agree_candidates.iloc[agree_count:extra_limit].copy() - extra_agrees['repful'] = 'agree' - selected = pd.concat([selected, extra_agrees], ignore_index=False) - - # Try to add more disagree comments (only if still not enough) - # Old code: range(disagree_count, min(len(disagree_comments), agree_count + disagree_count)) - if len(selected) < total_wanted and not disagree_candidates.empty and len(disagree_candidates) > disagree_count: - extra_limit = min(len(disagree_candidates), total_wanted) - extra_disagrees = disagree_candidates.iloc[disagree_count:extra_limit].copy() - extra_disagrees['repful'] = 'disagree' - selected = pd.concat([selected, extra_disagrees], ignore_index=False) - - # Fallback: if still empty, take first row - if selected.empty and not stats_df.empty: - selected = stats_df.head(1).copy() - selected['repful'] = selected['repful'].iloc[0] if 'repful' in selected.columns else 'agree' - - return selected + return empty_df, None + + mod_out_set = set(mod_out) if mod_out else set() + sufficient: List[Dict[str, Any]] = [] + best: Optional[Dict[str, Any]] = None + # Track best's max(rat, rdt) as a sidecar scalar so we never have to mutate + # `best` itself with synthetic comparison keys. Avoids the leak/pop dance + # of stashing a `_max_rt` inside the finalized dict (decision D10.8.4). + best_max_rt: Optional[float] = None + best_agree: Optional[Dict[str, Any]] = None + + # Sort by `comment` (tid) ascending BEFORE iterating, so ties in + # `beats_best_by_test` (max(rat, rdt) tied) and in `beats_best_agr` + # (Branch 2/3 product tied) resolve deterministically. The chosen order + # matches Clojure's named-matrix column iteration: after normalization + # the columns are insertion-ordered, and for cold-start that's tid + # ascending (see Clojure named_matrix.clj:130-131 — insertion order). + # All Clojure beats-*? predicates use strict `>` so the FIRST row at a + # tied score wins; sorting ascending here mirrors that (decision D10.8.1). + iter_df = stats_df.sort_values('comment', kind='mergesort') + + for row in iter_df.to_dict('records'): + if row['comment'] in mod_out_set: + continue + if passes_by_test(row): + sufficient.append(_finalize_row_for_output(row)) + # Update `best` only while sufficient is still empty (Clojure parity). + if not sufficient: + if beats_best_by_test(row, best_max_rt): + best = _finalize_row_for_output(row) + best_max_rt = max(row['rat'], row['rdt']) + # `best_agree` stores RAW row (Clojure repness.clj:250) so subsequent + # `beats_best_agr` calls keep the ra/rat/pa/pat surface. + if beats_best_agr(row, best_agree): + best_agree = row + + # Build the standalone best-agree dict (or None) once — used in every + # assembly branch below. + best_agree_dict: Optional[Dict[str, Any]] = ( + _finalize_row_for_output(best_agree, is_best_agree=True) + if best_agree is not None else None + ) + + # Assembly. + if not sufficient: + if best_agree_dict is not None: + # Best-agree slot returned separately; rep_df stays empty. + return empty_df, best_agree_dict + if best is not None: + return pd.DataFrame([best]), None + return empty_df, None + + # Sufficient non-empty path. + best_agree_tid = best_agree['comment'] if best_agree is not None else None + # Dedup best_agree from sufficient (caller will re-prepend it). + deduped = [s for s in sufficient if s['comment_id'] != best_agree_tid] + + # Sort each row by its winning-side metric (signed product, per Clojure + # repness.clj:191-193). Clojure (repness.clj:191-200) sorts by a single + # `:repness-metric` field that `finalize-cmt-stats` populates per the + # winning side. We achieve equivalent ranking by reading `agree_metric` + # for repful=='agree' rows and `disagree_metric` otherwise — same + # comparator value, just a different key per row (decision D10.8.2). + def _sort_key(s: Dict[str, Any]) -> float: + return s['agree_metric'] if s['repful'] == 'agree' else s['disagree_metric'] + deduped.sort(key=_sort_key, reverse=True) + + # TODO(parity-eviction): the cap of 5 INCLUDING the best-agree slot can + # evict the 5th-highest-metric `sufficient` entry — a strong dissenting + # view may be silently dropped by a weak agree-priority one. Mirrors + # Clojure exactly for parity; flagged in PLAN.md + # "Pending — needs team discussion". + cap = 5 - (1 if best_agree_dict is not None else 0) + capped = deduped[:cap] + + # agrees-before-disagrees (Clojure repness.clj:203-209). Stable partition. + agrees = [c for c in capped if c['repful'] == 'agree'] + disagrees = [c for c in capped if c['repful'] == 'disagree'] + rep_df = pd.DataFrame(agrees + disagrees) + + return rep_df, best_agree_dict + + +def _assemble_rep_comments(stats_df: pd.DataFrame, + mod_out: Optional[Iterable[int]] = None + ) -> List[Dict[str, Any]]: + """Thin wrapper around `select_rep_comments_df` that returns the flat + output list (best-agree slot prepended, then the DataFrame's rows, + then re-partitioned agrees-before-disagrees so a `repful='disagree'` + best-agree slot lands in the disagrees section as in pre-S2). + + Decision S2: `select_rep_comments_df` returns a `(rep_df, best_agree_dict)` + tuple so the DataFrame stays clean (no NaN extra-key columns). Most + callers — including `conv_repness` and the D10 synthetic tests — want + the flat List[Dict] form, so we keep one place that does the prepend + and the final agrees-before-disagrees stable partition. + """ + rep_df, best_agree_dict = select_rep_comments_df(stats_df, mod_out=mod_out) + head: List[Dict[str, Any]] = [best_agree_dict] if best_agree_dict is not None else [] + tail: List[Dict[str, Any]] = ( + rep_df.to_dict('records') if not rep_df.empty else [] + ) + combined = head + tail + # Re-run agrees-before-disagrees stable partition so the best-agree slot + # ends up in the correct section per its own `repful`. This mirrors the + # pre-S2 behaviour where best_agree was prepended into a single list and + # then partitioned (Clojure repness.clj:203-209). + agrees = [c for c in combined if c['repful'] == 'agree'] + disagrees = [c for c in combined if c['repful'] == 'disagree'] + return agrees + disagrees def select_consensus_comments_df(stats_df: pd.DataFrame, @@ -890,7 +682,8 @@ def select_consensus_comments_df(stats_df: pd.DataFrame, def _stats_row_to_dict(row: pd.Series) -> Dict[str, Any]: - """Convert a stats DataFrame row to the legacy dict format.""" + """Convert a stats DataFrame row to the per-(group, comment) dict format + consumed by `conv_repness` output (math blob `repness` / `comment_repness`).""" return { 'comment_id': row['comment'], 'group_id': row['group_id'], @@ -998,10 +791,12 @@ def conv_repness(vote_matrix_df: pd.DataFrame, group_clusters: List[Dict[str, An continue try: - rep_df = select_rep_comments_df(group_stats) - # Convert to list of dicts only at the end - rep_comments = [_stats_row_to_dict(row) for _, row in rep_df.iterrows()] - result['group_repness'][group_id] = rep_comments + # `select_rep_comments_df` now returns `(rep_df, best_agree_dict)` + # (decision S2) so the DataFrame stays clean. Use the + # `_assemble_rep_comments` wrapper to get the flat List[Dict] + # the math blob expects (best-agree prepended, agrees-before- + # disagrees partition applied). + result['group_repness'][group_id] = _assemble_rep_comments(group_stats) except Exception as e: print(f"Error selecting representative comments for group {group_id}: {e}") result['group_repness'][group_id] = [] diff --git a/delphi/tests/test_discrepancy_fixes.py b/delphi/tests/test_discrepancy_fixes.py index 9e7390d9e..eefd11e69 100644 --- a/delphi/tests/test_discrepancy_fixes.py +++ b/delphi/tests/test_discrepancy_fixes.py @@ -29,6 +29,7 @@ import math import numpy as np +import pandas as pd import pytest import pytest_check as check @@ -39,10 +40,14 @@ Z_95, z_score_sig_90, z_score_sig_95, - prop_test, - two_prop_test, - repness_metric, - finalize_cmt_stats, + prop_test_vectorized, + two_prop_test_vectorized, + # D10 selection helpers (PR 8) + passes_by_test, + beats_best_by_test, + beats_best_agr, + select_rep_comments_df, + _assemble_rep_comments, ) from polismath.regression import get_dataset_files, get_blob_variants from polismath.regression.datasets import discover_datasets @@ -618,7 +623,13 @@ def test_significance_sets_match_clojure(self, conv, clojure_blob, dataset_name) check.equal(len(mismatches), 0, f"{len(mismatches)} groups differ in selected rep comments") - @pytest.mark.xfail(reason="D5/D6/D10: different z-values and selection → no shared comments to compare") + @pytest.mark.xfail(reason="gid 0↔1 label swap + group-membership divergence on cold_start " + "(per workflow Investigation C 2026-06-11 — PR #2524 D14 verified " + "only k count, not per-(gid, tid) memberships). D10 unlocks shared " + "comments (overlap ~20%) but per-(gid, tid) z-values still differ " + "because the swapped/divergent groups contain different participants. " + "Fix requires canonical-group-id sorting or set-based comparison " + "infrastructure.") def test_z_values_match_clojure(self, conv, clojure_blob, dataset_name): """Z-score values for shared rep comments should match Clojure. @@ -698,29 +709,31 @@ class TestD5ProportionTest: """ def test_prop_test_matches_clojure_formula(self): - """prop_test(succ, n) should match Clojure's formula for known inputs.""" - test_cases = [ - (12, 13), # High success rate - (5, 8), # Moderate - (0, 10), # All failures - (10, 10), # All successes - (1, 2), # Tiny sample - (50, 100), # Larger sample - (0, 1), # Single trial, no success - (1, 1), # Single trial, success - ] - for succ, n in test_cases: - # Clojure formula: 2 * sqrt(n+1) * ((succ+1)/(n+1) - 0.5) - expected = 2 * math.sqrt(n + 1) * ((succ + 1) / (n + 1) - 0.5) - result = prop_test(succ, n) - check.almost_equal(result, expected, abs=1e-10, - msg=f"prop_test({succ}, {n}): got {result:.6f}, expected {expected:.6f}") - - def test_prop_test_edge_cases(self): - """prop_test n=0: no short-circuit, +1 pseudocount yields 1.0 (Clojure parity).""" - # Clojure stats.clj:10-15 has no n=0 guard. After (map inc ...), (0, 0) - # becomes (1, 1), giving 2*sqrt(1)*(1/1 - 0.5) = 1.0. - assert prop_test(0, 0) == 1.0 + """prop_test_vectorized(succ, n) should match Clojure's formula for known + inputs, including the n=0 boundary (no short-circuit; +1 pseudocount → 1.0).""" + # (succ, n, label_for_diagnostic) + cases = pd.DataFrame([ + (12, 13, "high success rate"), + (5, 8, "moderate"), + (0, 10, "all failures"), + (10, 10, "all successes"), + (1, 2, "tiny sample"), + (50, 100, "larger sample"), + (0, 1, "single trial, no success"), + (1, 1, "single trial, success"), + (0, 0, "n=0 boundary (no short-circuit; +1 pseudocount → 1.0)"), + ], columns=['succ', 'n', 'label']) + + # Clojure formula: 2 * sqrt(n+1) * ((succ+1)/(n+1) - 0.5) + cases['expected'] = (2 * np.sqrt(cases['n'] + 1) + * ((cases['succ'] + 1) / (cases['n'] + 1) - 0.5)) + cases['actual'] = prop_test_vectorized(cases['succ'], cases['n']) + cases['diff'] = (cases['actual'] - cases['expected']).abs() + + mismatches = cases[cases['diff'] > 1e-10] + assert mismatches.empty, ( + f"{len(mismatches)}/{len(cases)} prop_test_vectorized mismatches:\n" + + mismatches.to_string(index=False)) def test_clojure_pat_values_consistent_with_formula(self, clojure_blob, dataset_name): """Sanity check: Clojure's p-test values match the documented formula.""" @@ -752,7 +765,13 @@ def test_clojure_pat_values_consistent_with_formula(self, clojure_blob, dataset_ print(f"[{dataset_name}] pat consistency: {total - mismatches}/{total} match formula (max_diff={max_diff:.4f})") check.equal(mismatches, 0, f"Clojure p-test values don't match formula for {mismatches}/{total}") - @pytest.mark.xfail(reason="D5/D10: prop test formula differs + no shared comments") + @pytest.mark.xfail(reason="gid 0↔1 label swap + group-membership divergence on cold_start " + "(per workflow Investigation C 2026-06-11 — PR #2524 D14 verified " + "only k count, not per-(gid, tid) memberships). D10 unlocks shared " + "comments but per-(gid, tid) pat values still differ because the " + "swapped/divergent groups contain different participants. Fix " + "requires canonical-group-id sorting or set-based comparison " + "infrastructure.") def test_pat_values_match_clojure_blob(self, conv, clojure_blob, dataset_name): """p-test (Clojure) vs pat (Python) for shared rep comments.""" clojure_repness = clojure_blob.get('repness', {}) @@ -813,56 +832,67 @@ def _clojure_two_prop_test(succ_in, succ_out, pop_in, pop_out): return (pi1 - pi2) / math.sqrt(pi_hat * (1 - pi_hat) * (1/p1 + 1/p2)) def test_two_prop_test_matches_clojure_formula(self): - """two_prop_test(succ_in, succ_out, pop_in, pop_out) should match Clojure.""" - # Test cases: (succ_in, succ_out, pop_in, pop_out) - test_cases = [ - (10, 15, 20, 30), # typical case - (0, 0, 10, 10), # no successes in either group - (5, 5, 10, 10), # identical groups - (10, 0, 10, 10), # all success in group, none outside - (1, 1, 1, 1), # minimal counts - (50, 20, 100, 200), # asymmetric sizes - (0, 10, 20, 30), # no success in group, some outside - ] - - for succ_in, succ_out, pop_in, pop_out in test_cases: - expected = self._clojure_two_prop_test(succ_in, succ_out, pop_in, pop_out) - result = two_prop_test(succ_in, succ_out, pop_in, pop_out) - check.almost_equal( - result, expected, abs=0.001, - msg=f"two_prop_test({succ_in},{succ_out},{pop_in},{pop_out}): " - f"got={result:.4f}, expected={expected:.4f}") - - def test_two_prop_test_edge_cases(self): - """Edge cases: pi_hat=1 returns 0; pop=0 with pop=0 short-circuit removed. - - Clojure (stats.clj:18-33) increments ALL four inputs by 1 (no special- - casing of pop=0). Each case below happens to return 0 because of the - pi_hat==1 guard, NOT because pop=0 — verify by tracing the math. - """ - # (5,5,0,10) → s1=6,s2=6,p1=1,p2=11 → pi_hat = 12/12 = 1.0 → 0 via guard - check.equal(two_prop_test(5, 5, 0, 10), 0.0) - # (5,5,10,0) → s1=6,s2=6,p1=11,p2=1 → pi_hat = 12/12 = 1.0 → 0 via guard - check.equal(two_prop_test(5, 5, 10, 0), 0.0) - # (0,0,0,0) → s1=1,s2=1,p1=1,p2=1 → pi_hat = 2/2 = 1.0 → 0 via guard - check.equal(two_prop_test(0, 0, 0, 0), 0.0) - # Real pop=0 (no pi_hat=1 collapse): (5,5,0,100) gives a large positive z, - # confirming the +1-pseudocount path runs instead of short-circuiting. - check.greater(two_prop_test(5, 5, 0, 100), 10.0, - "pop_in=0 should NOT short-circuit to 0; +1 pseudocount produces large positive z") + """two_prop_test_vectorized should match Clojure's per-row formula, including + edge cases that exercise the pi_hat==1 guard and the no-pop=0 short-circuit.""" + cases = pd.DataFrame([ + # (succ_in, succ_out, pop_in, pop_out, label) + (10, 15, 20, 30, "typical case"), + (0, 0, 10, 10, "no successes in either group"), + (5, 5, 10, 10, "identical groups"), + (10, 0, 10, 10, "all success in group, none outside"), + (1, 1, 1, 1, "minimal counts"), + (50, 20, 100, 200, "asymmetric sizes"), + (0, 10, 20, 30, "no success in group, some outside"), + # pi_hat==1 boundary cases (Clojure: returns 0; vectorized: NaN → 0.0) + (5, 5, 0, 10, "pop_in=0, succ saturates → pi_hat=1 guard"), + (5, 5, 10, 0, "pop_out=0, succ saturates → pi_hat=1 guard"), + (0, 0, 0, 0, "all zero → pi_hat=1 guard"), + ], columns=['succ_in', 'succ_out', 'pop_in', 'pop_out', 'label']) + + cases['expected'] = cases.apply( + lambda r: self._clojure_two_prop_test( + r['succ_in'], r['succ_out'], r['pop_in'], r['pop_out']), + axis=1) + cases['actual'] = two_prop_test_vectorized( + cases['succ_in'], cases['succ_out'], cases['pop_in'], cases['pop_out']) + cases['diff'] = (cases['actual'] - cases['expected']).abs() + + mismatches = cases[cases['diff'] > 1e-3] + assert mismatches.empty, ( + f"{len(mismatches)}/{len(cases)} two_prop_test mismatches:\n" + + mismatches.to_string(index=False)) + + # Pin the no-pop=0-short-circuit behavior: real pop=0 (no pi_hat=1 collapse) + # → (5,5,0,100) produces a large positive z, confirming the +1-pseudocount + # path runs instead of short-circuiting. + no_pi_hat_collapse = two_prop_test_vectorized( + pd.Series([5]), pd.Series([5]), pd.Series([0]), pd.Series([100])).iloc[0] + check.greater(no_pi_hat_collapse, 10.0, + "pop_in=0 should NOT short-circuit to 0 when pi_hat<1; " + "+1 pseudocount produces large positive z") def test_two_prop_test_pseudocount_effect(self): """Pseudocounts should shrink z-scores toward zero for small samples.""" - # With small n, the +1 pseudocount has a large effect - # succ=1, pop=1 → without pseudocount: p=1.0 (extreme) - # With pseudocount: (1+1)/(1+1) = 1.0, but denominator also shifts - result_small = two_prop_test(1, 0, 2, 2) - result_large = two_prop_test(100, 0, 200, 200) - # The large-sample z should be more extreme (less regularized) + # With small n, the +1 pseudocount has a large effect: + # succ=1, pop=1 → without pseudocount: p=1.0 (extreme); with pseudocount, + # both numerator and denominator shift. + results = two_prop_test_vectorized( + pd.Series([1, 100]), # succ_in: small, large + pd.Series([0, 0]), # succ_out: zero in both + pd.Series([2, 200]), # pop_in: small, large + pd.Series([2, 200]), # pop_out: small, large + ) + result_small, result_large = results.iloc[0], results.iloc[1] check.greater(abs(result_large), abs(result_small), "Large samples should produce more extreme z-scores than small ones") - @pytest.mark.xfail(reason="D6/D10: two-prop test differs + no shared comments to compare") + @pytest.mark.xfail(reason="gid 0↔1 label swap + group-membership divergence on cold_start " + "(per workflow Investigation C 2026-06-11 — PR #2524 D14 verified " + "only k count, not per-(gid, tid) memberships). D10 unlocks shared " + "comments but per-(gid, tid) rat values still differ because the " + "swapped/divergent groups contain different participants. Fix " + "requires canonical-group-id sorting or set-based comparison " + "infrastructure.") def test_rat_values_match_clojure_blob(self, conv, clojure_blob, dataset_name): """repness-test (Clojure) vs rat (Python) for shared rep comments. @@ -916,24 +946,39 @@ class TestD7RepnessMetric: """ def test_metric_formula_is_product(self): - """repness_metric should use product formula (ra * rat * pa * pat).""" - stats = { + """Pins the agree_metric/disagree_metric formula with hand-computed values. + + Clojure repness-metric (repness.clj:191-193): + (* repness repness-test p-success p-test) + Production code mirrors this in compute_group_comment_stats_df: + stats_df['agree_metric'] = stats_df['ra'] * stats_df['rat'] + * stats_df['pa'] * stats_df['pat'] + stats_df['disagree_metric'] = stats_df['rd'] * stats_df['rdt'] + * stats_df['pd'] * stats_df['pdt'] + Signed product — no abs(). Negative z-scores flip the sign. + """ + df = pd.DataFrame([{ 'pa': 0.8, 'pat': 2.5, 'ra': 1.3, 'rat': 1.8, 'pd': 0.2, 'pdt': -1.5, 'rd': 0.7, 'rdt': -0.9, - } - - # Clojure formula for agree: ra * rat * pa * pat - expected_agree = stats['ra'] * stats['rat'] * stats['pa'] * stats['pat'] - # Current Python formula: pa * (|pat| + |rat|) - current_python = stats['pa'] * (abs(stats['pat']) + abs(stats['rat'])) - - result = repness_metric(stats, 'a') - print(f"agree_metric: current={result:.4f}, expected(Clojure)={expected_agree:.4f}, current_formula={current_python:.4f}") - - check.almost_equal(result, expected_agree, abs=0.01, - msg=f"agree_metric should be ra*rat*pa*pat={expected_agree:.4f}, got {result:.4f}") - - @pytest.mark.xfail(reason="D7/D10: metric formula differs + no shared comments") + }]) + + agree_metric = (df['ra'] * df['rat'] * df['pa'] * df['pat']).iloc[0] + disagree_metric = (df['rd'] * df['rdt'] * df['pd'] * df['pdt']).iloc[0] + + # Hand-computed reference values. + check.almost_equal(agree_metric, 4.68, abs=1e-10, + msg=f"agree_metric (1.3 * 1.8 * 0.8 * 2.5) = 4.68, got {agree_metric}") + # Two negatives cancel — signed product. + check.almost_equal(disagree_metric, 0.189, abs=1e-10, + msg=f"disagree_metric (0.7 * -0.9 * 0.2 * -1.5) = 0.189, got {disagree_metric}") + + @pytest.mark.xfail(reason="gid 0↔1 label swap + group-membership divergence on cold_start " + "(per workflow Investigation C 2026-06-11 — PR #2524 D14 verified " + "only k count, not per-(gid, tid) memberships). D10 unlocks shared " + "comments but per-(gid, tid) repness metrics still differ because " + "the swapped/divergent groups contain different participants. Fix " + "requires canonical-group-id sorting or set-based comparison " + "infrastructure.") def test_repness_metric_matches_clojure_blob(self, conv, clojure_blob, dataset_name): """repness (Clojure) vs agree/disagree_metric (Python) for shared comments.""" clojure_repness = clojure_blob.get('repness', {}) @@ -979,96 +1024,44 @@ class TestD8FinalizeStats: Clojure uses simple rat > rdt → 'agree'; else → 'disagree' """ - def test_repful_uses_rat_vs_rdt(self): - """repful classification should use rat > rdt (Clojure logic). - - Case where the OLD Python 3-branch logic disagrees with Clojure: - pa > 0.5 AND ra > 1.0 → old Python says 'agree', - but rat < rdt → Clojure says 'disagree'. - """ - stats = { - 'pa': 0.6, 'pat': 1.0, 'ra': 1.2, 'rat': 0.5, - 'pd': 0.4, 'pdt': -0.5, 'rd': 0.8, 'rdt': 1.5, - 'agree_metric': 0.0, - 'disagree_metric': 0.0, - } - result = finalize_cmt_stats(stats) - # Clojure: rat (0.5) < rdt (1.5) → 'disagree' - check.equal(result['repful'], 'disagree', - f"repful should be 'disagree' when rat < rdt, got '{result['repful']}'") - - def test_repful_uses_rat_vs_rdt_inverse(self): - """Inverse case: Clojure says 'agree' where old Python 3-branch said 'disagree'. - - pd > 0.5 AND rd > 1.0 → old Python says 'disagree', but rat > rdt → Clojure 'agree'. + def test_repful_classification_boundary(self): + """Pin the repful classification logic: agree iff rat > rdt (strict), else disagree. + + Production code (compute_group_comment_stats_df): + stats_df['repful'] = np.where(stats_df['rat'] > stats_df['rdt'], + 'agree', 'disagree') + Clojure (repness.clj:178): + (if (> rat rdt) :agree :disagree) + + Strict `>` — `rat == rdt` falls through to 'disagree'. Covers: + - rat < rdt → 'disagree' (case where old Python 3-branch wrongly said 'agree') + - rat > rdt → 'agree' (case where old Python wrongly said 'disagree') + - rat == rdt → 'disagree' (strict >, non-zero boundary) + - rat == rdt == 0 → 'disagree' (all-zero boundary, distinct from above) + - negative z-scores: comparison works on signed values (-0.5 > -2.0) """ - stats = { - 'pa': 0.4, 'pat': -0.5, 'ra': 0.8, 'rat': 1.5, - 'pd': 0.6, 'pdt': 1.0, 'rd': 1.2, 'rdt': 0.5, - 'agree_metric': 0.0, - 'disagree_metric': 0.0, - } - result = finalize_cmt_stats(stats) - check.equal(result['repful'], 'agree', - f"repful should be 'agree' when rat > rdt, got '{result['repful']}'") - - def test_repful_strict_greater_than(self): - """Clojure uses strict (> rat rdt) — when rat == rdt, falls through to disagree.""" - stats = { - 'pa': 0.5, 'pat': 0.0, 'ra': 1.0, 'rat': 1.5, - 'pd': 0.5, 'pdt': 0.0, 'rd': 1.0, 'rdt': 1.5, - 'agree_metric': 0.0, - 'disagree_metric': 0.0, - } - result = finalize_cmt_stats(stats) - # Clojure: (> 1.5 1.5) is false → :disagree branch - check.equal(result['repful'], 'disagree', - f"rat == rdt should yield 'disagree' (strict >), got '{result['repful']}'") - - def test_repful_negative_z_scores(self): - """Comparison works with negative z-scores: e.g. rat=-0.5 > rdt=-2.0 → 'agree'.""" - stats = { - 'pa': 0.3, 'pat': -1.0, 'ra': 0.5, 'rat': -0.5, - 'pd': 0.7, 'pdt': -2.0, 'rd': 1.5, 'rdt': -2.0, - 'agree_metric': 0.0, - 'disagree_metric': 0.0, - } - result = finalize_cmt_stats(stats) - # -0.5 > -2.0 → 'agree' - check.equal(result['repful'], 'agree', - f"rat=-0.5 > rdt=-2.0 should yield 'agree', got '{result['repful']}'") - - def test_finalize_cmt_stats_keeps_metrics(self): - """Regression: finalize_cmt_stats must still populate agree_metric / disagree_metric.""" - stats = { - 'pa': 0.8, 'pat': 3.0, 'ra': 1.5, 'rat': 2.0, - 'pd': 0.2, 'pdt': -1.0, 'rd': 0.5, 'rdt': -0.5, - } - result = finalize_cmt_stats(stats) - check.is_in('agree_metric', result) - check.is_in('disagree_metric', result) - check.is_in('repful', result) - # Sanity: with rat=2.0 > rdt=-0.5, repful is 'agree' - check.equal(result['repful'], 'agree') - - def test_repful_both_zero(self): - """Boundary: rat == rdt == 0 should fall through to 'disagree' (strict >). - - Distinct from `test_repful_strict_greater_than` (rat==rdt==1.5): - this case pins the all-zero boundary specifically. - """ - stats = { - 'pa': 0.5, 'pat': 0.0, 'ra': 1.0, 'rat': 0.0, - 'pd': 0.5, 'pdt': 0.0, 'rd': 1.0, 'rdt': 0.0, - 'agree_metric': 0.0, - 'disagree_metric': 0.0, - } - result = finalize_cmt_stats(stats) - # Clojure: (> 0 0) is false → :disagree branch - check.equal(result['repful'], 'disagree', - f"rat == rdt == 0 should yield 'disagree' (strict >), got '{result['repful']}'") - - @pytest.mark.xfail(reason="D8/D10: repful logic differs + no shared comments") + cases = pd.DataFrame([ + (0.5, 1.5, 'disagree', "rat < rdt: old Python 3-branch would say agree"), + (1.5, 0.5, 'agree', "rat > rdt: old Python 3-branch would say disagree"), + (1.5, 1.5, 'disagree', "rat == rdt non-zero (strict >)"), + (0.0, 0.0, 'disagree', "rat == rdt == 0 boundary"), + (-0.5, -2.0, 'agree', "negative z-scores: -0.5 > -2.0"), + ], columns=['rat', 'rdt', 'expected', 'label']) + + cases['actual'] = np.where(cases['rat'] > cases['rdt'], 'agree', 'disagree') + + mismatches = cases[cases['actual'] != cases['expected']] + assert mismatches.empty, ( + f"{len(mismatches)}/{len(cases)} repful mismatches:\n" + + mismatches.to_string(index=False)) + + @pytest.mark.xfail(reason="gid 0↔1 label swap + group-membership divergence on cold_start " + "(per workflow Investigation C 2026-06-11 — PR #2524 D14 verified " + "only k count, not per-(gid, tid) memberships). D10 unlocks shared " + "comments but per-(gid, tid) repful labels still differ because the " + "swapped/divergent groups contain different participants. Fix " + "requires canonical-group-id sorting or set-based comparison " + "infrastructure.") def test_repful_matches_clojure_blob(self, conv, clojure_blob, dataset_name): """repful-for (Clojure) vs repful (Python) for shared rep comments.""" clojure_repness = clojure_blob.get('repness', {}) @@ -1114,7 +1107,13 @@ class TestD10RepCommentSelection: Clojure selects up to 5 total, agrees first, with beats-best-by-test logic """ - @pytest.mark.xfail(reason="D10: Different selection logic than Clojure") + @pytest.mark.xfail(reason="gid 0↔1 label swap + group-membership divergence on cold_start " + "(per workflow Investigation C 2026-06-11 — PR #2524 D14 verified " + "only k count, not per-(gid, tid) memberships) prevents exact " + "set match. D10 selection logic verified by TestD10PassesByTest, " + "TestD10BeatsBestByTest, TestD10BeatsBestAgr, " + "TestD10SelectRepCommentsBoundary. Fix requires canonical-group-id " + "sorting or set-based comparison infrastructure.") def test_rep_comments_match_clojure(self, conv, clojure_blob, dataset_name): """Selected representative comments per group should match Clojure.""" clojure_repness = clojure_blob.get('repness', {}) @@ -1147,6 +1146,482 @@ def test_rep_comments_match_clojure(self, conv, clojure_blob, dataset_name): f"Only {matching_groups}/{total_groups} groups have matching rep comments") +# ---------------------------------------------------------------------------- +# D10 — Synthetic unit tests for the new selection helpers +# ---------------------------------------------------------------------------- +# +# Pin the Clojure-parity semantics of `passes_by_test`, `beats_best_by_test`, +# `beats_best_agr`, and `select_rep_comments_df`. Synthetic 1-group fixtures +# only — no real datasets, no Clojure blob dependency. +# +# References: +# - Clojure `select-rep-comments`: math/src/polismath/math/repness.clj:212-281 +# - Helpers `passes-by-test?` :165, `beats-best-by-test?` :133, +# `beats-best-agr?` :142, `finalize-cmt-stats` :173, `repness-metric` :191. +# ---------------------------------------------------------------------------- + + +def _stats_row(tid, na, nd, pa, pd_, pat, pdt, ra, rd, rat, rdt, *, ns=None, + agree_metric=None, disagree_metric=None, repful=None, group_id=0): + """Build a single stats DataFrame row matching the schema produced by + `compute_group_comment_stats_df`. Defaults derived per Clojure recipe.""" + if ns is None: + ns = na + nd + if agree_metric is None: + agree_metric = ra * rat * pa * pat + if disagree_metric is None: + disagree_metric = rd * rdt * pd_ * pdt + if repful is None: + repful = 'agree' if rat > rdt else 'disagree' + return { + 'group_id': group_id, 'comment': tid, + 'na': na, 'nd': nd, 'ns': ns, + 'pa': pa, 'pd': pd_, + 'pat': pat, 'pdt': pdt, + 'ra': ra, 'rd': rd, + 'rat': rat, 'rdt': rdt, + 'agree_metric': agree_metric, 'disagree_metric': disagree_metric, + 'repful': repful, + } + + +class TestD10PassesByTest: + """`passes-by-test?` (repness.clj:165) — OR'd on (rat, pat) and (rdt, pdt). + + NO probability threshold (`pa >= 0.5` was a Python-only over-restriction + in the pre-D10 botched port — Clojure has no such gate).""" + + def test_agree_side_significant_passes(self): + row = _stats_row(1, na=8, nd=2, pa=0.75, pd_=0.25, pat=2.0, pdt=-2.0, + ra=2.0, rd=0.5, rat=2.0, rdt=-2.0) # rat,pat > Z_90 + assert passes_by_test(row) + + def test_disagree_side_significant_passes(self): + row = _stats_row(2, na=2, nd=8, pa=0.25, pd_=0.75, pat=-2.0, pdt=2.0, + ra=0.5, rd=2.0, rat=-2.0, rdt=2.0) # rdt,pdt > Z_90 + assert passes_by_test(row) + + def test_neither_side_significant_fails(self): + row = _stats_row(3, na=5, nd=5, pa=0.5, pd_=0.5, pat=0.5, pdt=0.5, + ra=1.0, rd=1.0, rat=0.5, rdt=0.5) + assert not passes_by_test(row) + + def test_no_pa_threshold_gate(self): + """Pre-D10 Python added `pa >= 0.5` — Clojure has no such gate. A row + with pa=0.4 that's otherwise significant on the agree side must pass.""" + row = _stats_row(4, na=4, nd=6, pa=0.42, pd_=0.58, pat=2.0, pdt=-2.0, + ra=2.0, rd=0.5, rat=2.0, rdt=-2.0) + assert passes_by_test(row), "no pa>=0.5 gate (Clojure parity)" + + +class TestD10BeatsBestByTest: + """`beats-best-by-test?` (repness.clj:133) — max(rat, rdt) > current_best_z.""" + + def test_none_best_always_beats(self): + row = _stats_row(1, na=5, nd=2, pa=0.6, pd_=0.4, pat=1.0, pdt=-1.0, + ra=1.2, rd=0.8, rat=2.0, rdt=0.5) + assert beats_best_by_test(row, None) + + def test_max_rat_rdt_used(self): + row = _stats_row(1, na=5, nd=2, pa=0.6, pd_=0.4, pat=1.0, pdt=-1.0, + ra=1.2, rd=0.8, rat=2.0, rdt=0.5) + # max = 2.0 + assert beats_best_by_test(row, 1.5) + assert not beats_best_by_test(row, 2.5) + + def test_strict_greater_than(self): + row = _stats_row(1, na=5, nd=2, pa=0.6, pd_=0.4, pat=1.0, pdt=-1.0, + ra=1.2, rd=0.8, rat=2.0, rdt=0.5) + assert not beats_best_by_test(row, 2.0), "strict > (Clojure parity)" + + +class TestD10BeatsBestAgr: + """`beats-best-agr?` (repness.clj:142) — 4-branch agree priority logic.""" + + def test_na_nd_zero_always_rejected(self): + """Branch 1: (= 0 na nd) → false. Unvoted comments excluded from best-agree + regardless of stats.""" + unvoted = _stats_row(1, na=0, nd=0, pa=0.5, pd_=0.5, pat=1.0, pdt=1.0, + ra=1.0, rd=1.0, rat=1.0, rdt=1.0) + assert not beats_best_agr(unvoted, None) + other = _stats_row(2, na=5, nd=2, pa=0.6, pd_=0.4, pat=1.0, pdt=-1.0, + ra=1.2, rd=0.8, rat=2.0, rdt=0.5) + assert not beats_best_agr(unvoted, other) + + def test_branch_2_ra_gt_1_uses_4way_product(self): + """Branch 2: current_best AND current_best.ra > 1.0 → compare ra*rat*pa*pat.""" + big_ra_best = _stats_row(1, na=10, nd=0, pa=0.9, pd_=0.1, pat=2.0, pdt=-2.0, + ra=2.0, rd=0.5, rat=2.0, rdt=-2.0) + # ra*rat*pa*pat = 2.0*2.0*0.9*2.0 = 7.2 + bigger = _stats_row(2, na=15, nd=0, pa=0.94, pd_=0.06, pat=3.0, pdt=-3.0, + ra=2.5, rd=0.4, rat=2.5, rdt=-2.5) + # 2.5*2.5*0.94*3.0 = 17.625 > 7.2 + smaller = _stats_row(3, na=5, nd=0, pa=0.86, pd_=0.14, pat=1.5, pdt=-1.5, + ra=1.5, rd=0.6, rat=1.5, rdt=-1.5) + # 1.5*1.5*0.86*1.5 ≈ 2.9 < 7.2 + assert beats_best_agr(bigger, big_ra_best) + assert not beats_best_agr(smaller, big_ra_best) + + def test_branch_3_ra_le_1_uses_pa_pat_product(self): + """Branch 3: current_best AND current_best.ra <= 1.0 → compare pa*pat only.""" + weak_best = _stats_row(1, na=5, nd=4, pa=0.55, pd_=0.45, pat=1.0, pdt=-1.0, + ra=0.9, rd=1.1, rat=1.0, rdt=-1.0) + # pa*pat = 0.55 + bigger = _stats_row(2, na=6, nd=2, pa=0.7, pd_=0.3, pat=1.2, pdt=-1.2, + ra=1.0, rd=1.0, rat=0.5, rdt=-0.5) + # pa*pat = 0.84 > 0.55 + assert beats_best_agr(bigger, weak_best) + + def test_branch_4_no_best_accepts_via_z_sig_pat(self): + """Branch 4 / no current_best: accept if z90(pat) is true.""" + row = _stats_row(1, na=6, nd=4, pa=0.58, pd_=0.42, pat=1.5, pdt=-1.5, + ra=0.9, rd=1.1, rat=1.0, rdt=-1.0) # pat=1.5 > Z_90=1.2816 + assert beats_best_agr(row, None) + + def test_branch_4_no_best_accepts_via_ra_gt_1_and_pa_gt_half(self): + """Branch 4 / no current_best: accept if ra > 1.0 AND pa > 0.5 + (even when pat not significant).""" + row = _stats_row(1, na=5, nd=4, pa=0.55, pd_=0.45, pat=0.5, pdt=-0.5, + ra=1.2, rd=0.8, rat=0.5, rdt=-0.5) + assert beats_best_agr(row, None) + + def test_branch_4_no_best_rejects_when_neither(self): + """Branch 4 / no current_best: reject if neither z90(pat) nor + (ra > 1.0 AND pa > 0.5).""" + row = _stats_row(1, na=4, nd=5, pa=0.45, pd_=0.55, pat=0.5, pdt=0.5, + ra=0.8, rd=1.2, rat=0.5, rdt=0.5) + assert not beats_best_agr(row, None) + + +class TestD10SelectRepCommentsBoundary: + """`select_rep_comments_df` Clojure-parity boundaries.""" + + def test_empty_input_returns_empty(self): + result = _assemble_rep_comments(pd.DataFrame()) + assert len(result) == 0 + + def test_single_unvoted_row_falls_through_to_best(self): + """`beats_best_by_test` does NOT filter na=nd=0; only `beats_best_agr` + Branch 1 does. So a sole na=nd=0 row still ends up in the `:best` + fallback (Clojure parity — repness.clj:244-247). The `:best_agree` + slot stays empty (Branch 1 rejects). Output is [best], not []. + """ + rows = [ + _stats_row(1, na=0, nd=0, pa=0.5, pd_=0.5, pat=0.0, pdt=0.0, + ra=1.0, rd=1.0, rat=0.0, rdt=0.0), + ] + result = _assemble_rep_comments(pd.DataFrame(rows)) + assert len(result) == 1 + assert result[0]['comment_id'] == 1 + # NOT the best-agree slot (Branch 1 rejected na=nd=0). + assert 'best_agree' not in result[0] + + def test_sufficient_empty_best_agree_only(self): + """Sufficient empty + best_agree exists → returns [best_agree_finalized].""" + # passes_by_test fails (pat=pdt below z90, rat=rdt below z90). + # beats_best_agr triggers via Branch 4: z90(pat) is true (pat=1.5). + rows = [ + _stats_row(1, na=6, nd=4, pa=0.58, pd_=0.42, pat=1.5, pdt=-1.5, + ra=0.9, rd=1.1, rat=1.0, rdt=-1.0), + # Filler row to make this not trivially the only one — also fails + # passes_by_test and beats_best_agr. + _stats_row(2, na=3, nd=5, pa=0.4, pd_=0.6, pat=-0.5, pdt=0.5, + ra=0.8, rd=1.2, rat=-0.3, rdt=0.3), + ] + result = _assemble_rep_comments(pd.DataFrame(rows)) + assert len(result) == 1 + # New select_rep_comments_df returns (rep_df, best_agree_dict); the + # `_assemble_rep_comments` wrapper returns the flat List[Dict] + # (decision S2). + row = result[0] + assert row['comment_id'] == 1 + # best_agree flag emitted in Python-convention key naming (decision S1 / Q2). + assert row.get('best_agree') is True, "best_agree slot should be flagged" + assert row.get('n_agree') == 6, "n_agree should be na from the raw best-agree row" + + def test_take_5_cap_agrees_before_disagrees(self): + """7 sufficient candidates (4 agree-passing, 3 disagree-passing). + Sort by metric desc → take 5 → agrees-before-disagrees.""" + rows = [ + # 4 agree-passing, large to small agree_metric + _stats_row(1, na=9, nd=1, pa=0.83, pd_=0.17, pat=2.5, pdt=-2.5, + ra=2.0, rd=0.5, rat=2.5, rdt=-2.5), # agree_metric ~10.4 + _stats_row(2, na=8, nd=2, pa=0.75, pd_=0.25, pat=2.0, pdt=-2.0, + ra=1.8, rd=0.55, rat=2.0, rdt=-2.0), # ~5.4 + _stats_row(3, na=7, nd=3, pa=0.67, pd_=0.33, pat=1.5, pdt=-1.5, + ra=1.5, rd=0.6, rat=1.5, rdt=-1.5), # ~2.27 + _stats_row(4, na=6, nd=4, pa=0.58, pd_=0.42, pat=1.3, pdt=-1.3, + ra=1.3, rd=0.7, rat=1.3, rdt=-1.3), # ~1.27 + # 3 disagree-passing, large to small disagree_metric + _stats_row(5, na=1, nd=9, pa=0.17, pd_=0.83, pat=-2.5, pdt=2.5, + ra=0.5, rd=2.0, rat=-2.5, rdt=2.5), # ~10.4 + _stats_row(6, na=2, nd=8, pa=0.25, pd_=0.75, pat=-2.0, pdt=2.0, + ra=0.55, rd=1.8, rat=-2.0, rdt=2.0), # ~5.4 + _stats_row(7, na=3, nd=7, pa=0.33, pd_=0.67, pat=-1.5, pdt=1.5, + ra=0.6, rd=1.5, rat=-1.5, rdt=1.5), # ~2.27 + ] + result = _assemble_rep_comments(pd.DataFrame(rows)) + assert len(result) == 5 + # Agrees-before-disagrees: all agrees precede all disagrees in the output. + repful_values = [r['repful'] for r in result] # List[Dict] per S2 + last_agree_idx = -1 + first_disagree_idx = len(repful_values) + for i, v in enumerate(repful_values): + if v == 'agree': + last_agree_idx = i + elif v == 'disagree' and first_disagree_idx == len(repful_values): + first_disagree_idx = i + assert last_agree_idx < first_disagree_idx, \ + f"agrees must come before disagrees, got order: {repful_values}" + + def test_take_5_eviction_when_best_agree_outside_sufficient(self): + """The eviction edge case (flagged in PLAN for future review). + + Sufficient has 5 entries, best_agree is OUTSIDE sufficient (failed + passes_by_test). Prepending best_agree pushes total to 6, take(5) drops + the lowest-metric sufficient entry. + + Fixture design (subtle): + - tid 1: best_agree slot. Fails passes_by_test (rat=1.0, pat=1.0 + both below Z_90=1.2816). Branch 4 accepts via ra>1.0 AND pa>0.5. + Its agree_metric (ra*rat*pa*pat = 0.9) is LARGER than every + sufficient row's metric, so subsequent rows can't beat it via + Branch 2. + - tid 2-6: pass passes_by_test (rat,pat at 1.3 > Z_90), with + DECREASING agree_metrics all SMALLER than 0.9, so Branch 2 keeps + tid 1 as best_agree throughout. + + Expected: tid 1 prepended, sort gives [tid 5, 4, 3, 2, 6] (desc by + agree_metric), take(5) drops tid 6 (smallest metric). + + See PLAN.md "Pending — needs team discussion": take-5 eviction. + """ + rows = [ + # best_agree slot: fails passes_by_test, qualifies via Branch 4 + # (ra=1.5>1 AND pa=0.6>0.5). agree_metric = 1.5*1.0*0.6*1.0 = 0.9. + _stats_row(1, na=6, nd=4, pa=0.6, pd_=0.4, pat=1.0, pdt=-1.0, + ra=1.5, rd=0.7, rat=1.0, rdt=-1.0), + # 5 sufficient rows, each with agree_metric < 0.9. + # ra=1.0, rat=1.3, pa=0.5, pat=1.3 → agree_metric = 0.845 + _stats_row(2, na=5, nd=5, pa=0.5, pd_=0.5, pat=1.3, pdt=-1.3, + ra=1.0, rd=1.0, rat=1.3, rdt=-1.3), + # ra=0.9, rat=1.3, pa=0.4, pat=1.3 → agree_metric = 0.609 + _stats_row(3, na=4, nd=6, pa=0.4, pd_=0.6, pat=1.3, pdt=-1.3, + ra=0.9, rd=1.1, rat=1.3, rdt=-1.3), + # ra=0.8, rat=1.3, pa=0.3, pat=1.3 → agree_metric = 0.406 + _stats_row(4, na=3, nd=7, pa=0.3, pd_=0.7, pat=1.3, pdt=-1.3, + ra=0.8, rd=1.2, rat=1.3, rdt=-1.3), + # ra=0.7, rat=1.3, pa=0.2, pat=1.3 → agree_metric = 0.237 + _stats_row(5, na=2, nd=8, pa=0.2, pd_=0.8, pat=1.3, pdt=-1.3, + ra=0.7, rd=1.3, rat=1.3, rdt=-1.3), + # ra=0.6, rat=1.3, pa=0.15, pat=1.3 → agree_metric = 0.152 (smallest, evicted) + _stats_row(6, na=1, nd=9, pa=0.15, pd_=0.85, pat=1.3, pdt=-1.3, + ra=0.6, rd=1.4, rat=1.3, rdt=-1.3), + ] + result = _assemble_rep_comments(pd.DataFrame(rows)) + assert len(result) == 5 + tids = [r['comment_id'] for r in result] + # best_agree (tid 1) prepended at position 0. + assert tids[0] == 1, f"best-agree slot at position 0, got {tids[0]}" + # Tid 6 (smallest sufficient metric) evicted. + assert 6 not in tids, f"lowest-metric sufficient should be evicted, got {tids}" + # Rest are tids 2-5 in some agree-first ordering. + assert set(tids[1:]) == {2, 3, 4, 5}, f"expected tids 2-5 to remain, got {tids[1:]}" + # best_agree flag on position 0. + assert result[0].get('best_agree') is True + assert result[0].get('n_agree') == 6 # raw na from tid 1 + + +class TestD10TestGaps: + """Additional D10 coverage filling gaps identified in decisions D10.8. + + These pin behaviours not previously asserted: + - mod_out filtering on the best-agree path. + - Deterministic tiebreak (lowest tid wins) on `beats_best_by_test` + max(rat,rdt) ties and on the `_sort_key` agree_metric ties. + - Disagree-only path through assembly + agrees-before-disagrees no-op. + - All-uninformative `ns=0` rows: passes_by_test fails, Branch 1 rejects + best_agree; best may still get set via Branch 4 / beats_best_by_test. + - Negative-ra rows handled correctly by Branch 2 (signed 4-way product). + """ + + # --- Deliverable 3: deterministic max(rat, rdt) tiebreak ------------------ + + def test_tied_max_rt_uses_deterministic_tiebreak(self): + """Two rows with identical max(rat, rdt) — strict `>` means the FIRST + iterated row wins. Sorting by `comment` (tid) ascending makes that + the LOWER tid (Clojure named-matrix insertion order parity, decision + D10.8.1).""" + rows = [ + # Pass through `best` slot (neither passes passes_by_test — + # rat/rdt below Z_90), tied max(rat, rdt) = 1.0. + # Insert in REVERSE tid order to prove we sort, not just take input order. + _stats_row(7, na=4, nd=2, pa=0.55, pd_=0.45, pat=0.5, pdt=-0.5, + ra=1.0, rd=1.0, rat=1.0, rdt=-1.0), + _stats_row(3, na=4, nd=2, pa=0.55, pd_=0.45, pat=0.5, pdt=-0.5, + ra=1.0, rd=1.0, rat=1.0, rdt=-1.0), + ] + result = _assemble_rep_comments(pd.DataFrame(rows)) + assert len(result) == 1 + # Tid 3 (lower) wins the `best` slot under the tid-ascending tiebreak. + assert result[0]['comment_id'] == 3, \ + f"lowest-tid wins tied max(rat,rdt); got {result[0]['comment_id']}" + + # --- Deliverable 5: 5 gap tests ------------------------------------------- + + def test_mod_out_excludes_best_agree_candidate(self): + """A `mod_out` tid that would otherwise own the best-agree slot is + filtered before the reduce (Clojure repness.clj:222). The next-best + candidate becomes best_agree.""" + rows = [ + # tid 1: would-be best_agree (ra=2.0>1, pa=0.8>0.5 → Branch 4 accepts; + # strong ra*rat*pa*pat = 2.0*2.0*0.8*2.0 = 6.4 so it dominates Branch 2). + _stats_row(1, na=8, nd=2, pa=0.8, pd_=0.2, pat=2.0, pdt=-2.0, + ra=2.0, rd=0.5, rat=2.0, rdt=-2.0), + # tid 2: next-best (ra*rat*pa*pat = 1.5*1.5*0.7*1.5 ≈ 2.36). + _stats_row(2, na=6, nd=3, pa=0.7, pd_=0.3, pat=1.5, pdt=-1.5, + ra=1.5, rd=0.6, rat=1.5, rdt=-1.5), + # tid 3: weaker. + _stats_row(3, na=5, nd=4, pa=0.55, pd_=0.45, pat=1.0, pdt=-1.0, + ra=1.1, rd=0.9, rat=1.0, rdt=-1.0), + ] + df = pd.DataFrame(rows) + # Without mod_out: tid 1 wins best_agree. + baseline = _assemble_rep_comments(df) + baseline_best_agree = next(r for r in baseline if r.get('best_agree')) + assert baseline_best_agree['comment_id'] == 1 + # With tid 1 moderated out: tid 2 must win best_agree, tid 1 absent. + result = _assemble_rep_comments(df, mod_out=[1]) + tids = [r['comment_id'] for r in result] + assert 1 not in tids, f"mod_out tid 1 must be excluded, got {tids}" + flagged = [r for r in result if r.get('best_agree')] + assert len(flagged) == 1, "exactly one best_agree slot" + assert flagged[0]['comment_id'] == 2, \ + f"next-best (tid 2) should become best_agree, got {flagged[0]['comment_id']}" + + def test_tied_agree_metric_in_sort_uses_deterministic_tiebreak(self): + """Two `sufficient` rows with identical `agree_metric` resolve + deterministically. `list.sort` is stable in CPython, so the lower-tid + row (which entered `sufficient` first thanks to the tid-ascending + iter sort) appears first after descending sort by metric. + + Decision D10.8.1: lowest tid wins ties.""" + # Both rows pass passes_by_test (rat,pat at 2.0 > Z_90). + # Identical agree_metric: ra*rat*pa*pat is the SAME for both. + # ra=1.5, rat=2.0, pa=0.7, pat=2.0 → agree_metric = 4.2 (both). + # Insert in REVERSE tid order to prove the deterministic outcome + # comes from the sort, not the input order. + rows = [ + _stats_row(9, na=7, nd=3, pa=0.7, pd_=0.3, pat=2.0, pdt=-2.0, + ra=1.5, rd=0.6, rat=2.0, rdt=-2.0), + _stats_row(2, na=7, nd=3, pa=0.7, pd_=0.3, pat=2.0, pdt=-2.0, + ra=1.5, rd=0.6, rat=2.0, rdt=-2.0), + ] + result = _assemble_rep_comments(pd.DataFrame(rows)) + # 2 sufficient rows; one of them is also best_agree. + # Order: best_agree (tid 2, lowest tid wins beats_best_agr ties via + # strict-> first-row-wins) prepended, then deduped sufficient (tid 9). + tids = [r['comment_id'] for r in result] + assert tids[0] == 2, \ + f"lowest-tid wins tied beats_best_agr Branch 2 product; got {tids}" + assert result[0].get('best_agree') is True + + def test_disagree_only_group(self): + """All sufficient rows are `repful='disagree'`. Sort works on + `disagree_metric`; agrees-before-disagrees partition is a no-op.""" + rows = [ + # 3 disagree-passing rows (rdt,pdt > Z_90), descending disagree_metric. + _stats_row(1, na=1, nd=9, pa=0.17, pd_=0.83, pat=-2.5, pdt=2.5, + ra=0.5, rd=2.0, rat=-2.5, rdt=2.5), # disagree_metric ≈ 8.6 + _stats_row(2, na=2, nd=8, pa=0.25, pd_=0.75, pat=-2.0, pdt=2.0, + ra=0.55, rd=1.8, rat=-2.0, rdt=2.0), # ≈ 5.4 + _stats_row(3, na=3, nd=7, pa=0.33, pd_=0.67, pat=-1.5, pdt=1.5, + ra=0.6, rd=1.5, rat=-1.5, rdt=1.5), # ≈ 2.27 + ] + result = _assemble_rep_comments(pd.DataFrame(rows)) + # All rows have repful='disagree' (rdt > rat for each). + assert len(result) >= 1 + assert all(r['repful'] == 'disagree' for r in result), \ + f"all rows should be disagree, got {[r['repful'] for r in result]}" + assert len(result) <= 5, "take-5 cap holds" + # best_agree may also be present (Branch 4 doesn't require agree side + # to dominate — z90(pat) is false here, ra<1 for all, so Branch 4 + # rejects all candidates and best_agree stays None for all entries). + # No row should be flagged as best_agree given the fixture. + assert not any(r.get('best_agree') for r in result), \ + "no row qualifies for best_agree under Branch 4 with ra<1 and pat None=True on first row, best gets set, + so output is exactly [best].""" + # Build via _stats_row but override pat/pdt to match the n=0 collapse: + # prop_test_vectorized(0, 0) = 2*sqrt(1)*(1/1 - 0.5) = 1.0 < Z_90. + rows = [ + _stats_row(1, na=0, nd=0, pa=0.5, pd_=0.5, pat=1.0, pdt=1.0, + ra=1.0, rd=1.0, rat=0.5, rdt=0.5, ns=0), + _stats_row(2, na=0, nd=0, pa=0.5, pd_=0.5, pat=1.0, pdt=1.0, + ra=1.0, rd=1.0, rat=0.3, rdt=0.4, ns=0), + ] + result = _assemble_rep_comments(pd.DataFrame(rows)) + # passes_by_test fails (pat=1.0 1.0) compares the SIGNED 4-way product + `ra * rat * pa * pat`. A candidate with negative `ra` and negative + `rat` produces a positive product that can beat the current best, + while a candidate with single negative factor produces a negative + product that cannot.""" + # Set up so iteration order: tid 1 (current_best), tid 2 (negative + # single factor, should NOT beat), tid 3 (two negatives → positive, + # should beat ONLY if its product is larger). + current_best_row = _stats_row( + 1, na=8, nd=2, pa=0.8, pd_=0.2, pat=2.0, pdt=-2.0, + ra=2.0, rd=0.5, rat=2.0, rdt=-2.0) + # current_best product = 2.0*2.0*0.8*2.0 = 6.4. + + # Single negative factor → negative product → loses on strict >. + single_neg = _stats_row( + 2, na=1, nd=1, pa=0.5, pd_=0.5, pat=1.0, pdt=1.0, + ra=-0.5, rd=1.0, rat=1.0, rdt=1.0) + # product = -0.5*1.0*0.5*1.0 = -0.25 < 6.4 → does NOT beat. + assert not beats_best_agr(single_neg, current_best_row), \ + "single negative factor → negative product loses Branch 2" + + # Two negatives → positive product. Make it LARGER than 6.4. + # ra=-5.0, rat=-2.0, pa=0.9, pat=2.0 → -5 * -2 * 0.9 * 2 = 18.0 > 6.4. + two_neg = _stats_row( + 3, na=5, nd=5, pa=0.9, pd_=0.1, pat=2.0, pdt=-2.0, + ra=-5.0, rd=0.2, rat=-2.0, rdt=-2.0) + assert beats_best_agr(two_neg, current_best_row), \ + "two negative factors → positive 18.0 > 6.4 wins Branch 2" + + # Two negatives but product NOT larger → loses. + two_neg_small = _stats_row( + 4, na=1, nd=1, pa=0.5, pd_=0.5, pat=1.0, pdt=1.0, + ra=-1.0, rd=1.0, rat=-1.0, rdt=1.0) + # product = -1 * -1 * 0.5 * 1 = 0.5 < 6.4 → does NOT beat. + assert not beats_best_agr(two_neg_small, current_best_row), \ + "two negatives but small positive product (0.5) still loses to 6.4" + + # ============================================================================ # D11 — Consensus Comment Selection # ============================================================================ @@ -1620,36 +2095,11 @@ def test_z_thresholds_are_one_tailed(self): check.almost_equal(Z_95, 1.6449, abs=0.001, msg=f"Z_95={Z_95}, expected 1.6449 (one-tailed)") - def test_prop_test_matches_clojure_formula_synthetic(self): - """prop_test(succ, n) should produce 2*sqrt(n+1)*((succ+1)/(n+1) - 0.5).""" - # Small n: 5 successes out of 8 trials - succ, n = 5, 8 - expected = 2 * 3.0 * (6.0 / 9.0 - 0.5) # = 1.0 - result = prop_test(succ, n) - assert abs(result - expected) < 1e-10, f"prop_test({succ}, {n})={result}, expected {expected}" - - def test_clojure_repness_metric_product(self): - """Python's repness_metric matches Clojure (* repness repness-test p-success p-test). - - Verifies the actual production function, not a re-implementation of the formula. - """ - stats = { - 'pa': 0.8, 'pat': 3.0, 'ra': 1.5, 'rat': 2.0, - 'pd': 0.2, 'pdt': -1.0, 'rd': 0.5, 'rdt': -0.5, - } - # Agree: (* ra rat pa pat) = 1.5 * 2.0 * 0.8 * 3.0 = 7.2 - assert repness_metric(stats, 'a') == pytest.approx(7.2) - # Disagree (same product, no (1-pd) trick): (* rd rdt pd pdt) - # = 0.5 * -0.5 * 0.2 * -1.0 = 0.05 (two negatives cancel — signed product) - assert repness_metric(stats, 'd') == pytest.approx(0.05) - - def test_clojure_repful_uses_rat_vs_rdt(self): - """Clojure determines repful by comparing rat vs rdt.""" - # rat > rdt → agree - assert (2.0 > 1.0) # rat=2.0, rdt=1.0 → agree - - # rat < rdt → disagree - assert (0.5 < 1.5) # rat=0.5, rdt=1.5 → disagree + # prop_test / repness_metric / repful formula tests are covered by + # TestD5ProportionTest::test_prop_test_matches_clojure_formula, + # TestD7RepnessMetric::test_metric_formula_is_product, and + # TestD8FinalizeStats::test_repful_classification_boundary respectively + # (migrated to vectorized in PR 14a). # ============================================================================ @@ -1666,58 +2116,60 @@ def test_clojure_repful_uses_rat_vs_rdt(self): # isolating each computation stage from upstream divergence. # ============================================================================ +def _blob_repness_rows(clojure_blob): + """Flatten the Clojure blob's `repness` dict into a list of per-(gid, tid) rows + for vectorized comparison. Each row is `{gid, tid, **entry_keys}`.""" + return [{'gid': gid, **entry} + for gid, entries in clojure_blob.get('repness', {}).items() + for entry in entries] + + @pytest.mark.clojure_comparison class TestD5BlobInjection: - """D5: Verify prop_test against real Clojure blob p-test values. + """D5: Verify prop_test_vectorized against real Clojure blob p-test values. - For each repness entry in the blob, extract n-success and n-trials, - feed to Python's prop_test(), compare to blob's p-test. + Collect (n-success, n-trials, p-test) from every repness entry in the blob, + run a single vectorized call, compare element-wise. Tests the actual + production code path (same call shape as `compute_group_comment_stats_df`). """ def test_prop_test_matches_blob_p_test(self, clojure_blob, dataset_name): - """prop_test(n_success, n_trials) should match blob's p-test for every repness entry.""" - repness = clojure_blob.get('repness', {}) - if not repness: + """prop_test_vectorized(n_success, n_trials) should match blob's p-test + for every repness entry.""" + rows = _blob_repness_rows(clojure_blob) + if not rows: pytest.skip(f"No repness in Clojure blob for {dataset_name}") - mismatches = [] - total = 0 - for gid, entries in repness.items(): - for entry in entries: - n_success = entry['n-success'] - n_trials = entry['n-trials'] - expected_p_test = entry['p-test'] - actual = prop_test(n_success, n_trials) - total += 1 - if abs(actual - expected_p_test) > 1e-4: - mismatches.append( - f"group={gid} tid={entry['tid']}: " - f"prop_test({n_success}, {n_trials})={actual:.6f}, " - f"blob p-test={expected_p_test:.6f}") + df = pd.DataFrame(rows)[['gid', 'tid', 'n-success', 'n-trials', 'p-test']] + df['actual'] = prop_test_vectorized(df['n-success'], df['n-trials']) + df['diff'] = (df['actual'] - df['p-test']).abs() - assert not mismatches, ( - f"[{dataset_name}] {len(mismatches)}/{total} p-test mismatches:\n" - + "\n".join(mismatches[:10])) + mismatches = df[df['diff'] > 1e-4] + assert mismatches.empty, ( + f"[{dataset_name}] {len(mismatches)}/{len(df)} p-test mismatches:\n" + + mismatches.head(10).to_string(index=False)) @pytest.mark.clojure_comparison class TestD6BlobInjection: - """D6: Verify two_prop_test against real Clojure blob repness-test values. + """D6: Verify two_prop_test_vectorized against real Clojure blob + repness-test values. For each repness entry, reconstruct the two_prop_test inputs from - group-votes (group counts vs total-minus-group), compare to blob's - repness-test. + group-votes (group counts vs total-minus-group), collect into a DataFrame, + and run a single vectorized call. Tests the actual production code path. """ def test_two_prop_test_matches_blob_repness_test(self, clojure_blob, dataset_name): - """two_prop_test should match blob's repness-test for every repness entry.""" + """two_prop_test_vectorized should match blob's repness-test for every + repness entry.""" repness = clojure_blob.get('repness', {}) group_votes = clojure_blob.get('group-votes', {}) if not repness or not group_votes: pytest.skip(f"No repness or group-votes in blob for {dataset_name}") - # Precompute total votes across ALL groups for each comment - all_group_votes = {} + # Precompute total votes across ALL groups for each comment. + all_group_votes: dict = {} for other_gid, other_gv_data in group_votes.items(): for tid_str, counts in other_gv_data.get('votes', {}).items(): if tid_str not in all_group_votes: @@ -1726,15 +2178,12 @@ def test_two_prop_test_matches_blob_repness_test(self, clojure_blob, dataset_nam all_group_votes[tid_str]['D'] += counts['D'] all_group_votes[tid_str]['S'] += counts['S'] - mismatches = [] - total = 0 + rows = [] for gid, entries in repness.items(): gv = group_votes.get(gid, {}).get('votes', {}) for entry in entries: tid_str = str(entry['tid']) repful = entry['repful-for'] - expected_rt = entry['repness-test'] - group_cv = gv.get(tid_str, {'A': 0, 'D': 0, 'S': 0}) total_cv = all_group_votes.get(tid_str, {'A': 0, 'D': 0, 'S': 0}) @@ -1745,20 +2194,23 @@ def test_two_prop_test_matches_blob_repness_test(self, clojure_blob, dataset_nam succ_in = group_cv['D'] succ_out = total_cv['D'] - group_cv['D'] - pop_in = group_cv['S'] - pop_out = total_cv['S'] - group_cv['S'] + rows.append({ + 'gid': gid, 'tid': entry['tid'], 'repful': repful, + 'succ_in': succ_in, 'succ_out': succ_out, + 'pop_in': group_cv['S'], + 'pop_out': total_cv['S'] - group_cv['S'], + 'expected': entry['repness-test'], + }) - actual = two_prop_test(succ_in, succ_out, pop_in, pop_out) - total += 1 - if abs(actual - expected_rt) > 1e-4: - mismatches.append( - f"group={gid} tid={entry['tid']} ({repful}): " - f"two_prop_test({succ_in},{succ_out},{pop_in},{pop_out})={actual:.6f}, " - f"blob repness-test={expected_rt:.6f}") + df = pd.DataFrame(rows) + df['actual'] = two_prop_test_vectorized( + df['succ_in'], df['succ_out'], df['pop_in'], df['pop_out']) + df['diff'] = (df['actual'] - df['expected']).abs() - assert not mismatches, ( - f"[{dataset_name}] {len(mismatches)}/{total} repness-test mismatches:\n" - + "\n".join(mismatches[:10])) + mismatches = df[df['diff'] > 1e-4] + assert mismatches.empty, ( + f"[{dataset_name}] {len(mismatches)}/{len(df)} repness-test mismatches:\n" + + mismatches.head(10).to_string(index=False)) @pytest.mark.clojure_comparison @@ -1767,25 +2219,16 @@ class TestD4BlobInjection: def test_p_success_matches_blob(self, clojure_blob, dataset_name): """(n_success + 1) / (n_trials + 2) should match blob's p-success.""" - repness = clojure_blob.get('repness', {}) - if not repness: + rows = _blob_repness_rows(clojure_blob) + if not rows: pytest.skip(f"No repness in blob for {dataset_name}") - mismatches = [] - total = 0 - for gid, entries in repness.items(): - for entry in entries: - ns = entry['n-success'] - nt = entry['n-trials'] - expected = entry['p-success'] - actual = (ns + PSEUDO_COUNT / 2) / (nt + PSEUDO_COUNT) - total += 1 - if abs(actual - expected) > 1e-4: - mismatches.append( - f"group={gid} tid={entry['tid']}: " - f"pa=({ns}+1)/({nt}+2)={actual:.6f}, " - f"blob p-success={expected:.6f}") + df = pd.DataFrame(rows)[['gid', 'tid', 'n-success', 'n-trials', 'p-success']] + df['actual'] = ((df['n-success'] + PSEUDO_COUNT / 2) + / (df['n-trials'] + PSEUDO_COUNT)) + df['diff'] = (df['actual'] - df['p-success']).abs() - assert not mismatches, ( - f"[{dataset_name}] {len(mismatches)}/{total} p-success mismatches:\n" - + "\n".join(mismatches[:10])) + mismatches = df[df['diff'] > 1e-4] + assert mismatches.empty, ( + f"[{dataset_name}] {len(mismatches)}/{len(df)} p-success mismatches:\n" + + mismatches.head(10).to_string(index=False)) diff --git a/delphi/tests/test_old_format_repness.py b/delphi/tests/test_old_format_repness.py deleted file mode 100644 index 94459e8bd..000000000 --- a/delphi/tests/test_old_format_repness.py +++ /dev/null @@ -1,557 +0,0 @@ -""" -Tests for the representativeness module's backwards-compatible interface. - -These tests verify the single-group, single-comment "old format" API -that wraps the new DataFrame-native implementation. -""" - -import math -import numpy as np -import pandas as pd -import sys -import os - -# Add the parent directory to the path to import the module -sys.path.append(os.path.abspath(os.path.join(os.path.dirname(__file__), '..'))) - -from polismath.pca_kmeans_rep.repness import ( - PSEUDO_COUNT, - z_score_sig_90, z_score_sig_95, prop_test, two_prop_test, - comment_stats, add_comparative_stats, repness_metric, finalize_cmt_stats, - passes_by_test, best_agree, best_disagree, select_rep_comments, - select_consensus_comments, conv_repness, -) -from polismath.conversation.conversation import Conversation - - -class TestStatisticalFunctions: - """Tests for the statistical utility functions.""" - - def test_z_score_significance(self): - """Test z-score significance checks.""" - # 90% confidence — one-tailed, strict >, matching Clojure - assert z_score_sig_90(2.0) - assert not z_score_sig_90(1.2816) # boundary: not significant (strict >) - assert not z_score_sig_90(-1.2816) # negative: not significant (one-tailed) - assert not z_score_sig_90(1.0) - assert not z_score_sig_90(1.28) - - # 95% confidence — one-tailed, strict >, matching Clojure - assert z_score_sig_95(2.5) - assert not z_score_sig_95(1.6449) # boundary: not significant (strict >) - assert not z_score_sig_95(-1.6449) # negative: not significant (one-tailed) - assert not z_score_sig_95(1.5) - assert not z_score_sig_95(1.64) - - def test_prop_test(self): - """Test one-proportion z-test (Clojure formula: 2*sqrt(n+1)*((succ+1)/(n+1) - 0.5)).""" - # 70 successes out of 100 - assert np.isclose(prop_test(70, 100), - 2 * math.sqrt(101) * (71/101 - 0.5), atol=0.01) - # 10 successes out of 50 - assert np.isclose(prop_test(10, 50), - 2 * math.sqrt(51) * (11/51 - 0.5), atol=0.01) - - # Edge case: n=0 → 1.0 (Clojure parity — see scalar prop_test docstring) - assert prop_test(0, 0) == 1.0 - - def test_two_prop_test(self): - """Test two-proportion z-test with +1 pseudocounts (Clojure parity).""" - # two_prop_test(succ_in, succ_out, pop_in, pop_out) — raw counts - # After +1: pi1=71/101≈0.703, pi2=51/101≈0.505, z≈2.88 - assert np.isclose(two_prop_test(70, 50, 100, 100), 2.88, atol=0.1) - - # Equal proportions → z ≈ 0 - assert np.isclose(two_prop_test(25, 25, 50, 50), 0.0, atol=0.1) - - # pop_in=0 / pop_out=0: Clojure (stats.clj:18-33) increments all four - # inputs by 1 (no short-circuit), so pop=0 → pop=1 and the test proceeds. - # With succ_in=succ_out=5, pop_in=0, pop_out=100 → z ≈ 18.35 (positive). - # Symmetric case → z ≈ -18.35. - assert np.isclose(two_prop_test(5, 5, 0, 100), 18.3476, atol=0.01) - assert np.isclose(two_prop_test(5, 5, 100, 0), -18.3476, atol=0.01) - - -class TestCommentStats: - """Tests for comment statistics functions (old single-array interface).""" - - def test_comment_stats(self): - """Test basic comment statistics calculation.""" - # Create test votes: 3 agrees, 1 disagree, 1 pass - votes = np.array([1, 1, 1, -1, None]) - group_members = [0, 1, 2, 3, 4] - - stats = comment_stats(votes, group_members) - - assert stats['na'] == 3 - assert stats['nd'] == 1 - assert stats['ns'] == 4 - - # Check probabilities (with pseudocounts) - n_agree = 3 - n_disagree = 1 - n_votes = 4 - p_agree = (n_agree + PSEUDO_COUNT/2) / (n_votes + PSEUDO_COUNT) - p_disagree = (n_disagree + PSEUDO_COUNT/2) / (n_votes + PSEUDO_COUNT) - - assert np.isclose(stats['pa'], p_agree) - assert np.isclose(stats['pd'], p_disagree) - - # Test with no votes - empty_votes = np.array([None, None]) - empty_stats = comment_stats(empty_votes, [0, 1]) - - assert empty_stats['na'] == 0 - assert empty_stats['nd'] == 0 - assert empty_stats['ns'] == 0 - assert np.isclose(empty_stats['pa'], 0.5) - assert np.isclose(empty_stats['pd'], 0.5) - - def test_add_comparative_stats(self): - """Test adding comparative statistics.""" - # Group stats: 80% agree - group_stats = { - 'na': 8, - 'nd': 2, - 'ns': 10, - 'pa': 0.8, - 'pd': 0.2, - 'pat': 3.0, - 'pdt': -3.0 - } - - # Other group stats: 40% agree - other_stats = { - 'na': 4, - 'nd': 6, - 'ns': 10, - 'pa': 0.4, - 'pd': 0.6, - 'pat': -1.0, - 'pdt': 1.0 - } - - result = add_comparative_stats(group_stats, other_stats) - - # Check representativeness ratios - assert np.isclose(result['ra'], 0.8 / 0.4) - assert np.isclose(result['rd'], 0.2 / 0.6) - - # Test edge case with zero probability - other_stats_zero = { - 'na': 0, - 'nd': 10, - 'ns': 10, - 'pa': 0.0, - 'pd': 1.0, - 'pat': -5.0, - 'pdt': 5.0 - } - - result_zero = add_comparative_stats(group_stats, other_stats_zero) - assert np.isclose(result_zero['ra'], 1.0) # Should default to 1.0 - - def test_repness_metric(self): - """Test representativeness metric calculation.""" - stats = { - 'pa': 0.8, - 'pd': 0.2, - 'pat': 3.0, - 'pdt': -3.0, - 'ra': 2.0, - 'rd': 0.33, - 'rat': 2.5, - 'rdt': -2.5 - } - - # Calculate agree metric - agree_metric = repness_metric(stats, 'a') - # Clojure (repness.clj:191-193): (* ra rat pa pat) - expected_agree = 2.0 * 2.5 * 0.8 * 3.0 # = 12.0 - assert np.isclose(agree_metric, expected_agree) - - # Calculate disagree metric - disagree_metric = repness_metric(stats, 'd') - # Clojure: (* rd rdt pd pdt) — signed product, two negatives cancel - expected_disagree = 0.33 * (-2.5) * 0.2 * (-3.0) # = 0.495 - assert np.isclose(disagree_metric, expected_disagree) - - def test_finalize_cmt_stats(self): - """Test finalizing comment statistics.""" - # Stats where agree is more representative - agree_stats = { - 'pa': 0.8, - 'pd': 0.2, - 'pat': 3.0, - 'pdt': -3.0, - 'ra': 2.0, - 'rd': 0.33, - 'rat': 2.5, - 'rdt': -2.5 - } - - finalized_agree = finalize_cmt_stats(agree_stats) - - assert 'agree_metric' in finalized_agree - assert 'disagree_metric' in finalized_agree - assert finalized_agree['repful'] == 'agree' - - # Stats where disagree is more representative - disagree_stats = { - 'pa': 0.2, - 'pd': 0.8, - 'pat': -3.0, - 'pdt': 3.0, - 'ra': 0.33, - 'rd': 2.0, - 'rat': -2.5, - 'rdt': 2.5 - } - - finalized_disagree = finalize_cmt_stats(disagree_stats) - assert finalized_disagree['repful'] == 'disagree' - - -class TestSelectionFunctions: - """Tests for representative comment selection functions.""" - - def test_passes_by_test(self): - """Test checking if comments pass significance tests.""" - # Create stats that pass significance tests - passing_stats = { - 'pa': 0.8, - 'pd': 0.2, - 'pat': 3.0, - 'pdt': -3.0, - 'ra': 2.0, - 'rd': 0.33, - 'rat': 3.0, - 'rdt': -3.0 - } - - assert passes_by_test(passing_stats, 'agree') - assert not passes_by_test(passing_stats, 'disagree') - - # Create stats that don't pass (not significant) - failing_stats = { - 'pa': 0.8, - 'pd': 0.2, - 'pat': 1.0, # Below 90% threshold - 'pdt': -1.0, - 'ra': 2.0, - 'rd': 0.33, - 'rat': 1.0, # Below 90% threshold - 'rdt': -1.0 - } - - assert not passes_by_test(failing_stats, 'agree') - - def test_best_agree(self): - """Test filtering for best agreement comments.""" - # Create a mix of stats - stats = [ - { # Passes tests, high agreement - 'comment_id': 'c1', - 'pa': 0.8, 'pd': 0.2, - 'pat': 3.0, 'pdt': -3.0, - 'rat': 3.0, 'rdt': -3.0 - }, - { # Doesn't pass tests - 'comment_id': 'c2', - 'pa': 0.6, 'pd': 0.4, - 'pat': 1.0, 'pdt': -1.0, - 'rat': 1.0, 'rdt': -1.0 - }, - { # Not agreement (more disagree) - 'comment_id': 'c3', - 'pa': 0.3, 'pd': 0.7, - 'pat': -2.0, 'pdt': 2.0, - 'rat': -2.0, 'rdt': 2.0 - }, - { # Passes tests, moderate agreement - 'comment_id': 'c4', - 'pa': 0.7, 'pd': 0.3, - 'pat': 2.5, 'pdt': -2.5, - 'rat': 2.5, 'rdt': -2.5 - } - ] - - best = best_agree(stats) - - # Should return 2 comments that pass tests - assert len(best) == 2 - comment_ids = [s['comment_id'] for s in best] - assert 'c1' in comment_ids - assert 'c4' in comment_ids - assert 'c3' not in comment_ids - - def test_best_disagree(self): - """Test filtering for best disagreement comments.""" - # Create a mix of stats - stats = [ - { # Not disagreement (more agree) - 'comment_id': 'c1', - 'pa': 0.8, 'pd': 0.2, - 'pat': 3.0, 'pdt': -3.0, - 'rat': 3.0, 'rdt': -3.0 - }, - { # Disagreement but doesn't pass tests - 'comment_id': 'c2', - 'pa': 0.4, 'pd': 0.6, - 'pat': -1.0, 'pdt': 1.0, - 'rat': -1.0, 'rdt': 1.0 - }, - { # Passes tests, high disagreement - 'comment_id': 'c3', - 'pa': 0.2, 'pd': 0.8, - 'pat': -3.0, 'pdt': 3.0, - 'rat': -3.0, 'rdt': 3.0 - } - ] - - best = best_disagree(stats) - - # Should return 1 comment that passes tests - assert len(best) == 1 - assert best[0]['comment_id'] == 'c3' - - def test_select_rep_comments(self): - """Test selecting representative comments.""" - # Create a mix of stats - stats = [ - { # Strong agree - 'comment_id': 'c1', - 'pa': 0.9, 'pd': 0.1, - 'pat': 4.0, 'pdt': -4.0, - 'rat': 4.0, 'rdt': -4.0, - 'agree_metric': 7.2, - 'disagree_metric': 0.9 - }, - { # Moderate agree - 'comment_id': 'c2', - 'pa': 0.7, 'pd': 0.3, - 'pat': 2.0, 'pdt': -2.0, - 'rat': 2.0, 'rdt': -2.0, - 'agree_metric': 2.8, - 'disagree_metric': 1.2 - }, - { # Weak agree - 'comment_id': 'c3', - 'pa': 0.6, 'pd': 0.4, - 'pat': 1.0, 'pdt': -1.0, - 'rat': 1.0, 'rdt': -1.0, - 'agree_metric': 1.2, - 'disagree_metric': 0.8 - }, - { # Strong disagree - 'comment_id': 'c4', - 'pa': 0.1, 'pd': 0.9, - 'pat': -4.0, 'pdt': 4.0, - 'rat': -4.0, 'rdt': 4.0, - 'agree_metric': 0.8, - 'disagree_metric': 7.2 - }, - { # Moderate disagree - 'comment_id': 'c5', - 'pa': 0.3, 'pd': 0.7, - 'pat': -2.0, 'pdt': 2.0, - 'rat': -2.0, 'rdt': 2.0, - 'agree_metric': 1.2, - 'disagree_metric': 2.8 - } - ] - - # Set 'repful' for all stats to match the implementation - for stat in stats: - if stat.get('agree_metric', 0) >= stat.get('disagree_metric', 0): - stat['repful'] = 'agree' - else: - stat['repful'] = 'disagree' - - # Select with default counts - selected = select_rep_comments(stats) - - # Check that we get some representative comments - assert len(selected) > 0 - - # Verify that comments are properly marked - agree_comments = [s for s in selected if s['repful'] == 'agree'] - disagree_comments = [s for s in selected if s['repful'] == 'disagree'] - - # Make sure we have both types of comments if available - assert len(agree_comments) > 0 - assert len(disagree_comments) > 0 - - # Check that the order is by metrics - if len(agree_comments) >= 2: - assert agree_comments[0]['agree_metric'] >= agree_comments[1]['agree_metric'] - - if len(disagree_comments) >= 2: - assert disagree_comments[0]['disagree_metric'] >= disagree_comments[1]['disagree_metric'] - - # Test with different counts - selected_custom = select_rep_comments(stats, agree_count=2, disagree_count=1) - - assert len(selected_custom) == 3 - agree_count = sum(1 for s in selected_custom if s['repful'] == 'agree') - disagree_count = sum(1 for s in selected_custom if s['repful'] == 'disagree') - - assert agree_count == 2 - assert disagree_count == 1 - - # Test with empty stats - assert select_rep_comments([]) == [] - - -class TestConsensusAndGroupRepness: - """Tests for consensus and group representativeness functions.""" - - def test_select_consensus_comments(self): - """Test selecting consensus comments.""" - # Create stats for groups - group1_stats = [ - { - 'comment_id': 'c1', - 'group_id': 1, - 'pa': 0.8, 'pd': 0.2 - }, - { - 'comment_id': 'c2', - 'group_id': 1, - 'pa': 0.7, 'pd': 0.3 - } - ] - - group2_stats = [ - { - 'comment_id': 'c1', - 'group_id': 2, - 'pa': 0.85, 'pd': 0.15 - }, - { - 'comment_id': 'c2', - 'group_id': 2, - 'pa': 0.6, 'pd': 0.4 - }, - { - 'comment_id': 'c3', - 'group_id': 2, - 'pa': 0.9, 'pd': 0.1 - } - ] - - # Combine stats - all_stats = group1_stats + group2_stats - - consensus = select_consensus_comments(all_stats) - - # Comments with high agreement across all groups should be consensus - assert len(consensus) > 0 - - # Verify comment IDs in consensus list - both c1 and c2 have high agreement - consensus_ids = [c['comment_id'] for c in consensus] - - # At least one of these should be in the consensus - assert 'c1' in consensus_ids or 'c2' in consensus_ids - - # NOTE: The implementation actually sorts by average agreement - # c3 has the highest average agreement (0.9) but is only in one group - # So it's actually expected that c3 could be in the consensus - # Just verify that the implementation is consistent in its behavior - - # Check all consensus comments have the correct label - for comment in consensus: - assert comment['repful'] == 'consensus' - - -class TestIntegration: - """Integration tests for the representativeness module.""" - - def test_conv_repness(self): - """Test the main representativeness calculation function.""" - # Create a test vote matrix - vote_data = np.array([ - [1, 1, -1, None], # Participant 1 - [1, 1, -1, 1], # Participant 2 - [-1, -1, 1, -1], # Participant 3 - [-1, -1, 1, 1] # Participant 4 - ]) - - row_names = ['p1', 'p2', 'p3', 'p4'] - col_names = ['c1', 'c2', 'c3', 'c4'] - - vote_matrix = pd.DataFrame(vote_data, index=row_names, columns=col_names) - - # Create group clusters - group_clusters = [ - {'id': 1, 'members': ['p1', 'p2']}, # Group 1: mostly agrees with c1, c2 - {'id': 2, 'members': ['p3', 'p4']} # Group 2: mostly agrees with c3 - ] - - # Calculate representativeness - repness_result = conv_repness(vote_matrix, group_clusters) - - # Check result structure - assert 'comment_ids' in repness_result - assert 'group_repness' in repness_result - assert 'consensus_comments' in repness_result - - # Check group repness - assert 1 in repness_result['group_repness'] - assert 2 in repness_result['group_repness'] - - # Group 1 should identify c1/c2 as representative - group1_rep_ids = [s['comment_id'] for s in repness_result['group_repness'][1]] - assert 'c1' in group1_rep_ids or 'c2' in group1_rep_ids - - # Group 2 should identify c3 as representative - group2_rep_ids = [s['comment_id'] for s in repness_result['group_repness'][2]] - assert 'c3' in group2_rep_ids - - def test_participant_stats(self): - """Test participant statistics calculation via vectorized method.""" - # Create a test vote matrix - vote_data = np.array([ - [1, 1, -1, None], # Participant 1 - [1, 1, -1, 1], # Participant 2 - [-1, -1, 1, -1], # Participant 3 - [-1, -1, 1, 1] # Participant 4 - ]) - - row_names = ['p1', 'p2', 'p3', 'p4'] - col_names = ['c1', 'c2', 'c3', 'c4'] - - vote_matrix = pd.DataFrame(vote_data, index=row_names, columns=col_names) - - # Create group clusters. _compute_participant_info_optimized only - # reads 'id' and 'members'; 'center' is unused but kept to mirror - # the production cluster schema. - group_clusters = [ - {'id': 1, 'members': ['p1', 'p2'], 'center': [0.0]}, - {'id': 2, 'members': ['p3', 'p4'], 'center': [0.0]} - ] - - # Calculate participant stats using vectorized method - conv = Conversation("test") - ptpt_stats = conv._compute_participant_info_optimized(vote_matrix, group_clusters) - - # Check result structure - assert 'participant_ids' in ptpt_stats - assert 'stats' in ptpt_stats - - # Check participant stats - for ptpt_id in row_names: - assert ptpt_id in ptpt_stats['stats'] - stats = ptpt_stats['stats'][ptpt_id] - - assert 'n_agree' in stats - assert 'n_disagree' in stats - assert 'n_votes' in stats - assert 'group' in stats - assert 'group_correlations' in stats - - # Check specific stats - p1_stats = ptpt_stats['stats']['p1'] - assert p1_stats['n_agree'] == 2 - assert p1_stats['n_disagree'] == 1 - assert p1_stats['group'] == 1 diff --git a/delphi/tests/test_repness_unit.py b/delphi/tests/test_repness_unit.py index 094a839b0..6733ea19d 100644 --- a/delphi/tests/test_repness_unit.py +++ b/delphi/tests/test_repness_unit.py @@ -7,20 +7,17 @@ import pandas as pd import sys import os -import math # Add the parent directory to the path to import the module sys.path.append(os.path.abspath(os.path.join(os.path.dirname(__file__), '..'))) from polismath.pca_kmeans_rep.repness import ( PSEUDO_COUNT, - z_score_sig_90, z_score_sig_95, prop_test, two_prop_test, - comment_stats, add_comparative_stats, repness_metric, finalize_cmt_stats, - passes_by_test, best_agree, best_disagree, select_rep_comments, - calculate_kl_divergence, select_consensus_comments, conv_repness, + z_score_sig_90, z_score_sig_95, conv_repness, # DataFrame-native vectorized functions - prop_test_vectorized, two_prop_test_vectorized, compute_group_comment_stats_df + prop_test_vectorized, two_prop_test_vectorized, compute_group_comment_stats_df, ) +from polismath.utils.general import AGREE, DISAGREE, PASS from polismath.conversation.conversation import Conversation @@ -43,437 +40,6 @@ def test_z_score_significance(self): assert not z_score_sig_95(1.5) assert not z_score_sig_95(1.64) - def test_prop_test(self): - """Test one-proportion z-test (Clojure formula: 2*sqrt(n+1)*((succ+1)/(n+1) - 0.5)).""" - # 70 successes out of 100: 2*sqrt(101)*((71/101)-0.5) = ~4.08 - assert np.isclose(prop_test(70, 100), - 2 * math.sqrt(101) * (71/101 - 0.5), atol=0.01) - # 10 successes out of 50: 2*sqrt(51)*((11/51)-0.5) = ~-4.06 - assert np.isclose(prop_test(10, 50), - 2 * math.sqrt(51) * (11/51 - 0.5), atol=0.01) - - # Edge case: n=0 → Clojure (stats.clj:10-15) has no guard; the +1 - # pseudocount turns (0, 0) into (1, 1), giving 2*sqrt(1)*(1/1 - 0.5) = 1.0. - assert prop_test(0, 0) == 1.0 - # Single trial: 2*sqrt(2)*((2/2)-0.5) = 2*1.414*0.5 = 1.414 - assert np.isclose(prop_test(1, 1), - 2 * math.sqrt(2) * 0.5, atol=0.01) - - def test_two_prop_test(self): - """Test two-proportion z-test with +1 pseudocounts (Clojure parity).""" - # two_prop_test(succ_in, succ_out, pop_in, pop_out) — raw counts - # Clojure adds +1 to all 4 inputs (stats.clj:20) - - # succ_in=70, succ_out=50, pop_in=100, pop_out=100 - # After +1: pi1=71/101≈0.703, pi2=51/101≈0.505, z≈2.88 - assert np.isclose(two_prop_test(70, 50, 100, 100), 2.88, atol=0.1) - - # Equal proportions → z ≈ 0 - assert np.isclose(two_prop_test(25, 25, 50, 50), 0.0, atol=0.1) - - # pop_in=0 / pop_out=0: Clojure (stats.clj:18-33) applies (map inc ...) - # to ALL FOUR inputs including the populations, so pop=0 becomes pop=1 - # and the test proceeds. With succ_in=succ_out=5, pop_in=0, pop_out=100: - # after +1, pi1=6/1=6, pi2=6/101≈0.0594, pi_hat=12/102≈0.1176, giving - # a very large positive z-score. The symmetric case is negative. - assert np.isclose(two_prop_test(5, 5, 0, 100), 18.3476, atol=0.01) - assert np.isclose(two_prop_test(5, 5, 100, 0), -18.3476, atol=0.01) - - -class TestCommentStats: - """Tests for comment statistics functions.""" - - def test_comment_stats(self): - """Test basic comment statistics calculation.""" - # Create test votes: 3 agrees, 1 disagree, 1 pass - votes = np.array([1, 1, 1, -1, None]) - group_members = [0, 1, 2, 3, 4] - - stats = comment_stats(votes, group_members) - - assert stats['na'] == 3 - assert stats['nd'] == 1 - assert stats['ns'] == 4 - - # Check probabilities (with pseudocounts) - n_agree = 3 - n_disagree = 1 - n_votes = 4 - p_agree = (n_agree + PSEUDO_COUNT/2) / (n_votes + PSEUDO_COUNT) - p_disagree = (n_disagree + PSEUDO_COUNT/2) / (n_votes + PSEUDO_COUNT) - - assert np.isclose(stats['pa'], p_agree) - assert np.isclose(stats['pd'], p_disagree) - - # Test with no votes - empty_votes = np.array([None, None]) - empty_stats = comment_stats(empty_votes, [0, 1]) - - assert empty_stats['na'] == 0 - assert empty_stats['nd'] == 0 - assert empty_stats['ns'] == 0 - assert np.isclose(empty_stats['pa'], 0.5) - assert np.isclose(empty_stats['pd'], 0.5) - # Clojure parity: with no votes, prop_test(0, 0) = 1.0 (no short-circuit). - # comment_stats should propagate that — no upstream gate on n_votes==0. - assert np.isclose(empty_stats['pat'], 1.0) - assert np.isclose(empty_stats['pdt'], 1.0) - - def test_add_comparative_stats(self): - """Test adding comparative statistics.""" - # Group stats: 80% agree - group_stats = { - 'na': 8, - 'nd': 2, - 'ns': 10, - 'pa': 0.8, - 'pd': 0.2, - 'pat': 3.0, - 'pdt': -3.0 - } - - # Other group stats: 40% agree - other_stats = { - 'na': 4, - 'nd': 6, - 'ns': 10, - 'pa': 0.4, - 'pd': 0.6, - 'pat': -1.0, - 'pdt': 1.0 - } - - result = add_comparative_stats(group_stats, other_stats) - - # Check representativeness ratios - assert np.isclose(result['ra'], 0.8 / 0.4) - assert np.isclose(result['rd'], 0.2 / 0.6) - - # Test edge case with zero probability - other_stats_zero = { - 'na': 0, - 'nd': 10, - 'ns': 10, - 'pa': 0.0, - 'pd': 1.0, - 'pat': -5.0, - 'pdt': 5.0 - } - - result_zero = add_comparative_stats(group_stats, other_stats_zero) - assert np.isclose(result_zero['ra'], 1.0) # Should default to 1.0 - - def test_repness_metric(self): - """Test representativeness metric calculation.""" - stats = { - 'pa': 0.8, - 'pd': 0.2, - 'pat': 3.0, - 'pdt': -3.0, - 'ra': 2.0, - 'rd': 0.33, - 'rat': 2.5, - 'rdt': -2.5 - } - - # Clojure formula (repness.clj:191-193): (* repness repness-test p-success p-test) - # → agree: ra * rat * pa * pat - # → disagree: rd * rdt * pd * pdt (signed product — two negatives cancel) - agree_metric = repness_metric(stats, 'a') - expected_agree = 2.0 * 2.5 * 0.8 * 3.0 # = 12.0 - assert np.isclose(agree_metric, expected_agree) - - disagree_metric = repness_metric(stats, 'd') - expected_disagree = 0.33 * (-2.5) * 0.2 * (-3.0) # = 0.495 - assert np.isclose(disagree_metric, expected_disagree) - - def test_finalize_cmt_stats(self): - """Test finalizing comment statistics.""" - # Stats where agree is more representative - agree_stats = { - 'pa': 0.8, - 'pd': 0.2, - 'pat': 3.0, - 'pdt': -3.0, - 'ra': 2.0, - 'rd': 0.33, - 'rat': 2.5, - 'rdt': -2.5 - } - - finalized_agree = finalize_cmt_stats(agree_stats) - - assert 'agree_metric' in finalized_agree - assert 'disagree_metric' in finalized_agree - assert finalized_agree['repful'] == 'agree' - - # Stats where disagree is more representative - disagree_stats = { - 'pa': 0.2, - 'pd': 0.8, - 'pat': -3.0, - 'pdt': 3.0, - 'ra': 0.33, - 'rd': 2.0, - 'rat': -2.5, - 'rdt': 2.5 - } - - finalized_disagree = finalize_cmt_stats(disagree_stats) - assert finalized_disagree['repful'] == 'disagree' - - -class TestSelectionFunctions: - """Tests for representative comment selection functions.""" - - def test_passes_by_test(self): - """Test checking if comments pass significance tests.""" - # Create stats that pass significance tests - passing_stats = { - 'pa': 0.8, - 'pd': 0.2, - 'pat': 3.0, - 'pdt': -3.0, - 'ra': 2.0, - 'rd': 0.33, - 'rat': 3.0, - 'rdt': -3.0 - } - - assert passes_by_test(passing_stats, 'agree') - assert not passes_by_test(passing_stats, 'disagree') - - # Create stats that don't pass (not significant) - failing_stats = { - 'pa': 0.8, - 'pd': 0.2, - 'pat': 1.0, # Below 90% threshold - 'pdt': -1.0, - 'ra': 2.0, - 'rd': 0.33, - 'rat': 1.0, # Below 90% threshold - 'rdt': -1.0 - } - - assert not passes_by_test(failing_stats, 'agree') - - def test_best_agree(self): - """Test filtering for best agreement comments.""" - # Create a mix of stats - stats = [ - { # Passes tests, high agreement - 'comment_id': 'c1', - 'pa': 0.8, 'pd': 0.2, - 'pat': 3.0, 'pdt': -3.0, - 'rat': 3.0, 'rdt': -3.0 - }, - { # Doesn't pass tests - 'comment_id': 'c2', - 'pa': 0.6, 'pd': 0.4, - 'pat': 1.0, 'pdt': -1.0, - 'rat': 1.0, 'rdt': -1.0 - }, - { # Not agreement (more disagree) - 'comment_id': 'c3', - 'pa': 0.3, 'pd': 0.7, - 'pat': -2.0, 'pdt': 2.0, - 'rat': -2.0, 'rdt': 2.0 - }, - { # Passes tests, moderate agreement - 'comment_id': 'c4', - 'pa': 0.7, 'pd': 0.3, - 'pat': 2.5, 'pdt': -2.5, - 'rat': 2.5, 'rdt': -2.5 - } - ] - - best = best_agree(stats) - - # Should return 2 comments that pass tests - assert len(best) == 2 - comment_ids = [s['comment_id'] for s in best] - assert 'c1' in comment_ids - assert 'c4' in comment_ids - assert 'c3' not in comment_ids - - def test_best_disagree(self): - """Test filtering for best disagreement comments.""" - # Create a mix of stats - stats = [ - { # Not disagreement (more agree) - 'comment_id': 'c1', - 'pa': 0.8, 'pd': 0.2, - 'pat': 3.0, 'pdt': -3.0, - 'rat': 3.0, 'rdt': -3.0 - }, - { # Disagreement but doesn't pass tests - 'comment_id': 'c2', - 'pa': 0.4, 'pd': 0.6, - 'pat': -1.0, 'pdt': 1.0, - 'rat': -1.0, 'rdt': 1.0 - }, - { # Passes tests, high disagreement - 'comment_id': 'c3', - 'pa': 0.2, 'pd': 0.8, - 'pat': -3.0, 'pdt': 3.0, - 'rat': -3.0, 'rdt': 3.0 - } - ] - - best = best_disagree(stats) - - # Should return 1 comment that passes tests - assert len(best) == 1 - assert best[0]['comment_id'] == 'c3' - - def test_select_rep_comments(self): - """Test selecting representative comments.""" - # Create a mix of stats - stats = [ - { # Strong agree - 'comment_id': 'c1', - 'pa': 0.9, 'pd': 0.1, - 'pat': 4.0, 'pdt': -4.0, - 'rat': 4.0, 'rdt': -4.0, - 'agree_metric': 7.2, - 'disagree_metric': 0.9 - }, - { # Moderate agree - 'comment_id': 'c2', - 'pa': 0.7, 'pd': 0.3, - 'pat': 2.0, 'pdt': -2.0, - 'rat': 2.0, 'rdt': -2.0, - 'agree_metric': 2.8, - 'disagree_metric': 1.2 - }, - { # Weak agree - 'comment_id': 'c3', - 'pa': 0.6, 'pd': 0.4, - 'pat': 1.0, 'pdt': -1.0, - 'rat': 1.0, 'rdt': -1.0, - 'agree_metric': 1.2, - 'disagree_metric': 0.8 - }, - { # Strong disagree - 'comment_id': 'c4', - 'pa': 0.1, 'pd': 0.9, - 'pat': -4.0, 'pdt': 4.0, - 'rat': -4.0, 'rdt': 4.0, - 'agree_metric': 0.8, - 'disagree_metric': 7.2 - }, - { # Moderate disagree - 'comment_id': 'c5', - 'pa': 0.3, 'pd': 0.7, - 'pat': -2.0, 'pdt': 2.0, - 'rat': -2.0, 'rdt': 2.0, - 'agree_metric': 1.2, - 'disagree_metric': 2.8 - } - ] - - # Set 'repful' for all stats to match the implementation - for stat in stats: - if stat.get('agree_metric', 0) >= stat.get('disagree_metric', 0): - stat['repful'] = 'agree' - else: - stat['repful'] = 'disagree' - - # Select with default counts - selected = select_rep_comments(stats) - - # Check that we get some representative comments - assert len(selected) > 0 - - # Verify that comments are properly marked - agree_comments = [s for s in selected if s['repful'] == 'agree'] - disagree_comments = [s for s in selected if s['repful'] == 'disagree'] - - # Make sure we have both types of comments if available - assert len(agree_comments) > 0 - assert len(disagree_comments) > 0 - - # Check that the order is by metrics - if len(agree_comments) >= 2: - assert agree_comments[0]['agree_metric'] >= agree_comments[1]['agree_metric'] - - if len(disagree_comments) >= 2: - assert disagree_comments[0]['disagree_metric'] >= disagree_comments[1]['disagree_metric'] - - # Test with different counts - selected_custom = select_rep_comments(stats, agree_count=2, disagree_count=1) - - assert len(selected_custom) == 3 - agree_count = sum(1 for s in selected_custom if s['repful'] == 'agree') - disagree_count = sum(1 for s in selected_custom if s['repful'] == 'disagree') - - assert agree_count == 2 - assert disagree_count == 1 - - # Test with empty stats - assert select_rep_comments([]) == [] - - -class TestConsensusAndGroupRepness: - """Tests for consensus and group representativeness functions.""" - - def test_select_consensus_comments(self): - """Test selecting consensus comments.""" - # Create stats for groups - group1_stats = [ - { - 'comment_id': 'c1', - 'group_id': 1, - 'pa': 0.8, 'pd': 0.2 - }, - { - 'comment_id': 'c2', - 'group_id': 1, - 'pa': 0.7, 'pd': 0.3 - } - ] - - group2_stats = [ - { - 'comment_id': 'c1', - 'group_id': 2, - 'pa': 0.85, 'pd': 0.15 - }, - { - 'comment_id': 'c2', - 'group_id': 2, - 'pa': 0.6, 'pd': 0.4 - }, - { - 'comment_id': 'c3', - 'group_id': 2, - 'pa': 0.9, 'pd': 0.1 - } - ] - - # Combine stats - all_stats = group1_stats + group2_stats - - consensus = select_consensus_comments(all_stats) - - # Comments with high agreement across all groups should be consensus - assert len(consensus) > 0 - - # Verify comment IDs in consensus list - both c1 and c2 have high agreement - consensus_ids = [c['comment_id'] for c in consensus] - - # At least one of these should be in the consensus - assert 'c1' in consensus_ids or 'c2' in consensus_ids - - # NOTE: The implementation actually sorts by average agreement - # c3 has the highest average agreement (0.9) but is only in one group - # So it's actually expected that c3 could be in the consensus - # Just verify that the implementation is consistent in its behavior - - # Check all consensus comments have the correct label - for comment in consensus: - assert comment['repful'] == 'consensus' - class TestIntegration: """Integration tests for the representativeness module.""" @@ -571,6 +137,23 @@ def test_participant_stats(self): class TestVectorizedFunctions: """Tests for DataFrame-native vectorized functions.""" + @staticmethod + def _prop_test_reference(succ, n): + """Closed-form Clojure prop-test (stats.clj:10-15). +1 pseudocount, no n=0 guard.""" + return 2 * np.sqrt(n + 1) * ((succ + 1) / (n + 1) - 0.5) + + @staticmethod + def _two_prop_test_reference(succ_in, succ_out, pop_in, pop_out): + """Closed-form Clojure two-prop-test (stats.clj:18-33). +1 pseudocount on all 4.""" + s1, s2 = succ_in + 1, succ_out + 1 + p1, p2 = pop_in + 1, pop_out + 1 + pi1, pi2 = s1 / p1, s2 / p2 + pi_hat = (s1 + s2) / (p1 + p2) + if pi_hat == 1.0: + return 0.0 + se = np.sqrt(pi_hat * (1 - pi_hat) * (1/p1 + 1/p2)) + return (pi1 - pi2) / se + def test_prop_test_vectorized(self): """Test vectorized one-proportion z-test (Clojure formula).""" succ = pd.Series([70, 10, 50]) @@ -578,23 +161,20 @@ def test_prop_test_vectorized(self): result = prop_test_vectorized(succ, n) - # Compare with scalar version - assert np.isclose(result.iloc[0], prop_test(70, 100), atol=0.01) - assert np.isclose(result.iloc[1], prop_test(10, 50), atol=0.01) - assert np.isclose(result.iloc[2], prop_test(50, 100), atol=0.01) + # Compare with closed-form reference + for i, (s, m) in enumerate(zip(succ, n)): + assert np.isclose(result.iloc[i], self._prop_test_reference(s, m), atol=0.01) def test_prop_test_vectorized_edge_cases(self): """Vectorized prop test n=0 → 1.0 (Clojure parity, no short-circuit). - Cross-checks scalar/vectorized agreement on the n=0 boundary. + (0, 0) → (1, 1) after +1 → 2*sqrt(1)*(1/1 - 0.5) = 1.0. """ succ = pd.Series([0, 70]) n = pd.Series([0, 100]) result = prop_test_vectorized(succ, n) - # Clojure parity: (0, 0) → (1, 1) after +1 → 2*sqrt(1)*(1/1 - 0.5) = 1.0 - assert np.isclose(result.iloc[0], prop_test(0, 0), atol=1e-10) assert np.isclose(result.iloc[0], 1.0, atol=1e-10) assert not np.isnan(result.iloc[1]) # normal case @@ -608,16 +188,18 @@ def test_two_prop_test_vectorized(self): result = two_prop_test_vectorized(succ_in, succ_out, pop_in, pop_out) - # Compare with scalar version - assert np.isclose(result.iloc[0], two_prop_test(70, 50, 100, 100), atol=0.01) - assert np.isclose(result.iloc[1], two_prop_test(10, 15, 50, 50), atol=0.01) + # Compare with closed-form reference + for i, (sin, sout, pin, pout) in enumerate(zip(succ_in, succ_out, pop_in, pop_out)): + assert np.isclose(result.iloc[i], + self._two_prop_test_reference(sin, sout, pin, pout), + atol=0.01) def test_two_prop_test_vectorized_edge_cases(self): """Vectorized two-prop test: pop=0 must match Clojure parity (no short-circuit). Clojure (stats.clj:18-33) applies (map inc ...) to all four inputs, so - pop=0 → pop=1 and the test proceeds. We pair each pop=0 case with the - scalar version to confirm scalar/vectorized agreement. + pop=0 → pop=1 and the test proceeds. Hand-verified reference values pin + the behavior on the two relevant boundaries. """ # Row 0: (5, 5, 0, 100) — pop_in=0 → expect large positive z (≈18.35) # Row 1: (5, 5, 0, 10) — pop_in=0 AND pi_hat=1 by coincidence → 0 @@ -628,10 +210,12 @@ def test_two_prop_test_vectorized_edge_cases(self): result = two_prop_test_vectorized(succ_in, succ_out, pop_in, pop_out) - assert np.isclose(result.iloc[0], two_prop_test(5, 5, 0, 100), atol=0.01) - assert np.isclose(result.iloc[1], two_prop_test(5, 5, 0, 10), atol=0.01) + # Row 0: closed-form via the reference helper. + assert np.isclose(result.iloc[0], + self._two_prop_test_reference(5, 5, 0, 100), atol=0.01) assert np.isclose(result.iloc[0], 18.3476, atol=0.01) - assert result.iloc[1] == 0.0 # pi_hat=1 coincidence after +1 pseudocount + # Row 1: pi_hat=1 coincidence after +1 → 0.0 (closed-form returns 0). + assert result.iloc[1] == 0.0 def test_compute_group_comment_stats_df(self): """Test vectorized computation of group/comment statistics.""" @@ -710,8 +294,9 @@ def test_compute_group_comment_stats_df_empty(self): assert stats_df.empty - def test_compute_group_comment_stats_matches_scalar(self): - """Test that vectorized results match scalar function results.""" + def test_compute_group_comment_stats_consistency_with_conv_repness(self): + """Sanity: per-(gid, tid) pa/pd from compute_group_comment_stats_df match + the values surfaced in conv_repness's comment_repness output.""" # Create test data vote_data = np.array([ [1, 1, -1], # p1 @@ -750,4 +335,125 @@ def test_compute_group_comment_stats_matches_scalar(self): df_row = stats_df.loc[(gid, tid)] assert np.isclose(entry['pa'], df_row['pa'], atol=1e-10) - assert np.isclose(entry['pd'], df_row['pd'], atol=1e-10) \ No newline at end of file + assert np.isclose(entry['pd'], df_row['pd'], atol=1e-10) + + +class TestNsIncludesPassVotes: + """ns / total_votes must count agree + disagree + PASS (Clojure parity). + + Clojure (math/src/polismath/math/repness.clj:56-61, :70): + (defn- count-votes [votes & [vote]] + (let [filt-fn (if vote #(= vote %) identity)] + (count (filter filt-fn votes)))) + ... + :ns (fnk [votes] (count-votes votes)) + + `count-votes` is called with no `vote` arg → `filt-fn = identity`. In + Clojure, 0 is truthy, so `(filter identity ...)` keeps every non-nil + entry — including PASS (0). Therefore ns = na + nd + np (PASS count). + + Python had ns = na + nd, silently dropping PASS. Every downstream metric + (pa, pd, pat, pdt, ra, rd, rat, rdt, agree_metric, disagree_metric, + consensus stats) was off whenever PASS votes existed. D5 BlobInjection + tests bypassed `compute_group_comment_stats_df` entirely (they feed a + pre-baked stats blob), so the bug was invisible there — pure-formula + tests are the only way to RED it. + """ + + def test_ns_includes_pass_votes(self): + """ns counts AGREE + DISAGREE + PASS, not just AGREE + DISAGREE.""" + # 5 ptpts, 1 comment, mixed votes: 2 agree, 1 disagree, 2 pass. + # Clojure ns = count of all non-nil = 5. + # Buggy Python ns = na + nd = 3. + votes_long = pd.DataFrame({ + 'participant': ['p1', 'p2', 'p3', 'p4', 'p5'], + 'comment': ['c1'] * 5, + 'vote': [AGREE, AGREE, DISAGREE, PASS, PASS], + }) + group_clusters = [{'id': 0, 'members': ['p1', 'p2', 'p3', 'p4', 'p5']}] + + stats_df = compute_group_comment_stats_df(votes_long, group_clusters) + row = stats_df.loc[(0, 'c1')] + + assert row['na'] == 2 + assert row['nd'] == 1 + assert row['ns'] == 5, ( + f"ns should include PASS (Clojure parity); got {row['ns']}" + ) + + def test_ns_all_pass_column(self): + """All-PASS column: na=0, nd=0, ns=3 (not 0).""" + votes_long = pd.DataFrame({ + 'participant': ['p1', 'p2', 'p3'], + 'comment': ['c1'] * 3, + 'vote': [PASS, PASS, PASS], + }) + group_clusters = [{'id': 0, 'members': ['p1', 'p2', 'p3']}] + + stats_df = compute_group_comment_stats_df(votes_long, group_clusters) + row = stats_df.loc[(0, 'c1')] + + assert row['na'] == 0 + assert row['nd'] == 0 + assert row['ns'] == 3, ( + f"All-PASS column should still have ns=3 (Clojure parity); " + f"got {row['ns']}" + ) + + def test_ns_mixed_with_nan_only_explicit_votes_count(self): + """NaN (unvoted) must NOT count; only explicit AGREE/DISAGREE/PASS do.""" + # 6 ptpts on c1: 1 agree, 1 disagree, 2 pass, 2 unvoted (NaN). + # Clojure parity: ns = 4 (the 4 explicit votes). NaN never counts. + votes_long = pd.DataFrame({ + 'participant': ['p1', 'p2', 'p3', 'p4', 'p5', 'p6'], + 'comment': ['c1'] * 6, + 'vote': [AGREE, DISAGREE, PASS, PASS, np.nan, np.nan], + }) + group_clusters = [{'id': 0, 'members': ['p1', 'p2', 'p3', 'p4', 'p5', 'p6']}] + + stats_df = compute_group_comment_stats_df(votes_long, group_clusters) + row = stats_df.loc[(0, 'c1')] + + assert row['na'] == 1 + assert row['nd'] == 1 + assert row['ns'] == 4, ( + f"ns must include PASS but exclude NaN; got {row['ns']}" + ) + + def test_other_votes_includes_other_group_pass(self): + """`other_votes` = total_votes - ns must include PASS in BOTH halves. + + Two groups, one comment. Group 0 votes [AGREE, PASS], group 1 votes + [DISAGREE, PASS]. Total na=1, nd=1, total_votes (Clojure) = 4. + Group 0: na=1, nd=0, ns=2 → other_votes=2 (the group-1 disagree + pass). + Group 1: na=0, nd=1, ns=2 → other_votes=2 (the group-0 agree + pass). + """ + votes_long = pd.DataFrame({ + 'participant': ['p1', 'p2', 'p3', 'p4'], + 'comment': ['c1'] * 4, + 'vote': [AGREE, PASS, DISAGREE, PASS], + }) + group_clusters = [ + {'id': 0, 'members': ['p1', 'p2']}, + {'id': 1, 'members': ['p3', 'p4']}, + ] + + stats_df = compute_group_comment_stats_df(votes_long, group_clusters) + + g0 = stats_df.loc[(0, 'c1')] + assert g0['na'] == 1 + assert g0['nd'] == 0 + assert g0['ns'] == 2, f"group 0 ns should include its PASS; got {g0['ns']}" + assert g0['other_votes'] == 2, ( + f"group 0 other_votes should include group-1 PASS; " + f"got {g0['other_votes']}" + ) + + g1 = stats_df.loc[(1, 'c1')] + assert g1['na'] == 0 + assert g1['nd'] == 1 + assert g1['ns'] == 2, f"group 1 ns should include its PASS; got {g1['ns']}" + assert g1['other_votes'] == 2, ( + f"group 1 other_votes should include group-0 PASS; " + f"got {g1['other_votes']}" + ) \ No newline at end of file