diff --git a/delphi/docs/CLJ-PARITY-FIXES-JOURNAL.md b/delphi/docs/CLJ-PARITY-FIXES-JOURNAL.md index 629179770..fa9e110ef 100644 --- a/delphi/docs/CLJ-PARITY-FIXES-JOURNAL.md +++ b/delphi/docs/CLJ-PARITY-FIXES-JOURNAL.md @@ -1216,3 +1216,455 @@ 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. + +## Session: PR 9 — D11 consensus comment selection (2026-06-11) + +Landed in `/goal` mode. Decisions documented in +`~/polis/D10_D11_D12_GOLDENS_DECISIONS.md` (D11.x section). + +### What landed + +**Production (`delphi/polismath/pca_kmeans_rep/repness.py`):** +- New `consensus_stats_df(vote_matrix_df, mod_out=None) -> pd.DataFrame`: + whole-conversation per-comment stats (no group split, no `ra/rd/rat/rdt`). + Vectorized port of Clojure `consensus-stats` (repness.clj:284-290). +- Rewrite `select_consensus_comments_df(cons_stats) -> Dict[str, List[Dict]]`: + matches Clojure `select-consensus-comments` (repness.clj:293-323). + Filters: agree `pa > 0.5 AND z-sig-90(pat)`, disagree + `pd > 0.5 AND z-sig-90(pdt)`. Ordering: descending `pa*pat` / `pd*pdt`. + Cap: top 5 each side. Output: `{'agree': [...], 'disagree': [...]}`. +- `conv_repness` grows `mod_out: Optional[Iterable[int]] = None` kwarg. + Forwarded to both `select_rep_comments_df` and `consensus_stats_df`. +- Consensus is now computed unconditionally (Clojure parity — pre-D11 + Python had a `len(group_clusters) > 1` guard with no Clojure analog). +- `_stats_row_to_dict` deleted (orphan after D11). + +**Caller (`conversation.py`):** +- `_compute_repness` passes `mod_out=self.mod_out_tids` to `conv_repness`. + +**Downstream consumers updated** for the new dict shape: +- `tests/test_repness_smoke.py::test_repness_structure` — iterates + `consensus['agree']` and `consensus['disagree']`. +- `tests/test_pipeline_integrity.py::test_full_pipeline` — same. + +**Tests (12 new in `tests/test_discrepancy_fixes.py`):** +- `TestD11ConsensusStatsDf` (4): basic counts, pseudocount pa/pd, + ns=0 fallback, mod_out filter. +- `TestD11SelectConsensusBoundary` (8): empty input, clear agree + consensus, clear disagree consensus, divisive (no consensus), top-5 + cap, entry keys (Python convention per S1), disagree entry key + mapping (n_success ← nd, p_success ← pd, p_test ← pdt), + mutually-exclusive agree/disagree lists. + +### Suite delta + +- Pre (post-D10): 313 passed, 12 skipped, 58 xfailed. +- Post (this PR): 325 passed, 12 skipped, 58 xfailed. +- Delta: +12 (the 12 new D11 synthetic tests). Zero regressions. + +### DISCOVERY: ns-PASS divergence + +The D11 real-data test (`test_consensus_matches_clojure`) showed 3-5/5 +overlap on cold_start — close but not exact. Investigation revealed a +deeper bug: + +**Clojure's `:ns`** (via `count-votes` with `filter identity` — +repness.clj:56-61) INCLUDES PASS votes (`0` is truthy in Clojure). + +**Python's `ns`** in BOTH `compute_group_comment_stats_df` and the new +`consensus_stats_df` computes `ns = na + nd`, EXCLUDING PASS. + +This means every downstream metric (pa, pd, pat, pdt, ra, rd, rat, rdt, +agree_metric, disagree_metric) is computed with the wrong denominator +when PASS votes are present. The D5 PR #2519 journal claim that "PASS NOT +included, matching Clojure" was a misreading of `count-votes`. + +**Impact:** +- D5/D6/D7/D8 blob-comparison tests' "mismatches" were not (only) + upstream PCA/KMeans divergence — the ns-PASS divergence is at least + a contributing cause. +- D11 consensus partial overlap is consistent with this divergence. +- Fixing requires a separate PR affecting two production functions and + re-recording goldens. + +D11 real-data test xfailed with the right reason. Logic pinned by the +12 synthetic tests (which never exercise PASS, so they don't show the +divergence). + +This is now the top item under "Pending — needs team discussion" in +PLAN.md, with a sketch of the fix. + +### What's Next + +PR 11 (D12) on top of D11 in the same spr stack. + +## Session: PR 11 — D12 comment priorities (2026-06-11) + +Landed in `/goal` mode. Decisions documented in +`~/polis/D10_D11_D12_GOLDENS_DECISIONS.md` (D12.x section). + +### What landed + +**`pca.py`:** +- `pca_project_cmnts(center, comps) -> np.ndarray`: vectorized projection + of each comment into 2D PCA space. Closed-form derivation: + `proj[i] = -sqrt(n_cmnts) * (1 + center[i]) * [pc1[i], pc2[i]]`. +- `compute_comment_extremity(cmnt_proj) -> np.ndarray`: L2 norm per row. + +Clojure parity: `pca-project-cmnts` (pca.clj:167-178) + +`with-proj-and-extremtiy` (conversation.clj:341-352). + +**`conversation.py` module-level:** +- `META_PRIORITY = 7` constant (Clojure conversation.clj:319). +- `importance_metric(A, P, S, E) -> float`: Clojure conversation.clj:311-315. +- `priority_metric(is_meta, A, P, S, E) -> float`: Clojure conversation.clj:321-330. + Squared formula. For meta: `META_PRIORITY^2 = 49`. For non-meta: + `(importance * (1 + 8*2^(-S/5)))^2` where the decay factor lets new + (low-S) comments bubble up. + +**`Conversation._compute_comment_priorities()`:** +- Computes comment projection + extremity from PCA. +- Aggregates A/D/S across all groups per tid; derives P = S - (A + D). +- Looks up extremity per tid (via `self.rating_mat.columns` column order). +- Checks `tid in self.meta_tids` for the meta branch. +- Stores `{tid: priority_float}` on `self.comment_priorities`. + +Wired into `recompute()` after `_compute_repness()`. The serialization +infrastructure (`to_dict`, `to_dynamo_dict`, underscore→hyphen conversion) +already existed but was emitting empty. + +**B1 + B2 fixes from D11 sub-agent review folded in:** +- B1: `conversation.py:834` no-groups early-return now emits + `consensus_comments: {'agree': [], 'disagree': []}` (dict) instead of `[]`. +- B2: `test_legacy_repness_comparison.py:197` updated to read the dict + shape + flatten for ID extraction. + +### Tests (11 new + 1 xfail flipped + 2 xpassed = 14 new+repurposed) + +- `TestD12PCAProjectComments` (5): output shape, formula verification, empty + input, L2 extremity, empty extremity. +- `TestD12PriorityMetrics` (6): importance formula vs Clojure ref values + (conversation.clj:335), high-extremity boosts, meta constant=49, non-meta + squared formula, decay-factor lets-new-bubble-up, META_PRIORITY=7. +- `TestD12CommentPriorities::test_comment_priorities_exist` xfail dropped + (existed pre-PR), then re-xfailed for a different reason: Clojure blob + has constant priorities (all 49.0 = META_PRIORITY^2) on vw/biodiversity + → Spearman comparison meaningless. + +### DISCOVERY: Clojure blob has all-meta priorities + +`vw-cold_start`: ALL 125 tids have priority = 49.0 in Clojure blob. +`biodiversity-cold_start`: ALL 314 tids have priority = 49.0. + +Either: +- (a) Every tid was meta-tagged in those Clojure runs. +- (b) Clojure's `(if 0 ...)` truthiness quirk: 0 is truthy in Clojure, so + ANY value (even `0`) returned by `(get meta-tids tid 0)` triggers the + meta branch. + +Python correctly distinguishes meta from non-meta via Boolean set membership, +producing varied priorities 0.18-31.46. + +Logged for batch review. Python may be more correct than Clojure here. + +### Suite delta + +- Pre (post-D11): 325 passed, 12 skipped, 58 xfailed. +- Post (this PR): 336 passed, 12 skipped, 56 xfailed, 2 xpassed. +- Delta: +11 (the 11 new D12 synthetic tests), 0 failed, -2 xfailed + (those became xpassed — the 2 cold_start `test_comment_priorities_exist` + variants run cleanly now; the new xfail is on a different basis). + +### What's Next + +Re-record vw + biodiversity Python golden snapshots (PR-stack tip). diff --git a/delphi/docs/PLAN_DISCREPANCY_FIXES.md b/delphi/docs/PLAN_DISCREPANCY_FIXES.md index d2df7b48d..36d00c0e4 100644 --- a/delphi/docs/PLAN_DISCREPANCY_FIXES.md +++ b/delphi/docs/PLAN_DISCREPANCY_FIXES.md @@ -28,10 +28,11 @@ 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 9 (D11) | — (WIP) | — | Fix D11: consensus selection — **NEEDS REWORK** | +| PR 14a (scalar deletion) | #2564 | — | Delete dead scalar paths in `repness.py`; migrate blob injection tests to vectorized | +| PR 8 (D10) | #2566 | — | Fix D10: rep comment selection — single-pass reduce matching Clojure | +| PR 9 (D11) | #2567 | — | Fix D11: consensus selection — whole-conv stats + per-side top-5 matching Clojure | | PR 10 (D3) | — (WIP) | — | Fix D3: k-smoother buffer — **NEEDS REWORK** | -| PR 11 (D12) | — (WIP) | — | Fix D12: comment priorities — **NEEDS REWORK** | +| PR 11 (D12) | — (in flight) | — | Fix D12: comment priorities — Clojure-parity importance/priority metrics + PCA comment projection | | PR 13 (D1) | — (WIP) | — | Fix D1: PCA sign flip prevention — **NEEDS REWORK** | | PR 15 | — (WIP) | — | Fix load_votes timestamp ordering — **NEEDS REWORK** | @@ -914,3 +915,30 @@ 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. +- **`ns` includes-PASS-divergence** (DISCOVERED 2026-06-11 during D11). Clojure's + `:ns` (via `count-votes` with `filter identity` — repness.clj:56-61) INCLUDES + PASS votes. Python's `compute_group_comment_stats_df` and `consensus_stats_df` + both compute `ns = na + nd`, excluding PASS. This is a real divergence that + affects `pa, pd, pat, pdt, ra, rd, rat, rdt` everywhere — every downstream + metric and selection. The D5 PR #2519 journal claim ("PASS NOT included, + matching Clojure") was based on a misreading of `count-votes`. Currently + causing 3-5% divergence in pat values for tids with non-zero PASS counts; + visible at the consensus-selection margins (4/6 overlap on vw cold_start + agree, 1/3 on disagree). Needs a dedicated PR — affects: + - `compute_group_comment_stats_df` (line ~283: `ns = na + nd`). + - `consensus_stats_df` (line ~435: `ns = na + nd`). + Fix: `ns = (vote_matrix_df != 0).sum(axis=0)` no, actually we want to + count non-NaN: `ns = vote_matrix_df.notna().sum(axis=0)` for wide format; + for long-format `votes_long.groupby('comment').size()` after dropna. + Re-record goldens afterward. +- **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/conversation/conversation.py b/delphi/polismath/conversation/conversation.py index b6f2178a8..b9c3f0eac 100644 --- a/delphi/polismath/conversation/conversation.py +++ b/delphi/polismath/conversation/conversation.py @@ -15,7 +15,11 @@ from datetime import datetime from natsort import natsorted -from polismath.pca_kmeans_rep.pca import pca_project_dataframe +from polismath.pca_kmeans_rep.pca import ( + pca_project_dataframe, + pca_project_cmnts, + compute_comment_extremity, +) from polismath.pca_kmeans_rep.clusters import ( kmeans_sklearn, calculate_silhouette_sklearn @@ -37,6 +41,88 @@ logger.setLevel(logging.INFO) +# ============================================================================= +# D12: Comment-priority metrics (Clojure parity) +# ============================================================================= +# +# Ports of `importance-metric` and `priority-metric` from Clojure +# (math/src/polismath/math/conversation.clj:311-330). Public so they can be +# unit-tested in isolation. + +META_PRIORITY = 7 # Clojure: meta-priority (conversation.clj:319). "TODO TUNE." + + +def importance_metric(A: float, P: float, S: float, E: float) -> float: + """ + Clojure importance-metric (conversation.clj:311-315). + + (defn importance-metric + [A P S E] + (let [p (/ (+ P 1) (+ S 2)) + a (/ (+ A 1) (+ S 2))] + (* (- 1 p) (+ E 1) a))) + + Smoothed (Beta(2,2)) probability of pass `p`, smoothed agree `a`, with + extremity boost `(E + 1)`. Higher when fewer passes, more agrees, more + extreme (higher PCA extremity). + + Args: + A: agree count (across all groups). + P: pass count = S - (A + D) across all groups. + S: seen count (total votes seen — agree + disagree + pass). + E: comment extremity (L2 norm of PCA projection). + """ + p = (P + 1) / (S + 2) + a = (A + 1) / (S + 2) + return (1 - p) * (E + 1) * a + + +def priority_metric(is_meta: bool, + A: float, P: float, S: float, E: float) -> float: + """ + Clojure priority-metric (conversation.clj:321-330). + + (defn priority-metric + [is-meta A P S E] + (matrix/pow + (if is-meta + meta-priority + (* (importance-metric A P S E) + (+ 1 (* 8 (matrix/pow 2 (/ S -5)))))) + 2)) + + Squared to deepen bias (toward extremes). Meta comments get a constant + `META_PRIORITY^2 = 49`. Non-meta comments get `importance * decay`, where + the decay factor `1 + 8 * 2^(-S/5)` lets new (low-S) comments bubble up + and fades as more votes accumulate. + + Args: + is_meta: True for meta comments (treated as constant priority). + A, P, S, E: see `importance_metric`. + + Returns: + Squared priority value. + """ + # TODO(clojure-parity-bug): Clojure (conversation.clj:325) treats meta-tid + # value 0 as TRUTHY in (if is-meta ...), so every tid takes the meta branch. + # We mirror this bug for byte-for-byte Clojure parity. Switch back to + # honoring `is_meta` once the GitHub issue resolves: + # https://github.com/compdemocracy/polis/issues/2571 + # Original semantic-correct code preserved below for reference and future + # restoration. + # + # Clojure-parity-bug-mirror: ALWAYS take the meta branch, ignoring is_meta. + return META_PRIORITY ** 2 + + # Original semantically-correct logic, restore when Clojure bug is fixed: + # if is_meta: + # inner = META_PRIORITY + # else: + # decay_factor = 1 + 8 * (2 ** (-S / 5)) + # inner = importance_metric(A, P, S, E) * decay_factor + # return inner ** 2 + + class Conversation: """ Manages the state and computation for a Pol.is conversation. @@ -82,6 +168,7 @@ def __init__(self, self.participant_info = {} self.vote_stats = {} self.group_votes = {} # Initialize group_votes to avoid attribute errors + self.comment_priorities: Dict[Any, float] = {} # D12 (PR 11) # Initialize with votes if provided if votes: @@ -753,16 +840,22 @@ def _compute_repness(self) -> None: # Check if we have groups if not self.group_clusters: + # B1 fix (D11 sub-agent review): consensus_comments must always be + # `{'agree': [], 'disagree': []}` (dict) post-D11, never `[]` (list). self.repness = { 'comment_ids': list(self.rating_mat.columns), 'group_repness': {}, - 'consensus_comments': [] + 'consensus_comments': {'agree': [], 'disagree': []} } logger.info(f"Representativeness completed in {time.time() - start_time:.2f}s (no groups)") return - # Compute representativeness (needs participant IDs, not base-cluster IDs) - self.repness = conv_repness(self.rating_mat, self._unfolded_group_clusters()) + # Compute representativeness (needs participant IDs, not base-cluster IDs). + # `mod_out=self.mod_out_tids` forwards moderated-out tids to the rep + consensus + # selectors (Clojure parity per D11 / PR 9; matches repness.clj:222 and :296). + self.repness = conv_repness(self.rating_mat, + self._unfolded_group_clusters(), + mod_out=self.mod_out_tids) logger.info(f"Representativeness completed in {time.time() - start_time:.2f}s") def _compute_participant_info_optimized(self, vote_matrix: pd.DataFrame, group_clusters: List[Dict[str, Any]]) -> Dict[str, Any]: @@ -1001,11 +1094,78 @@ def recompute(self) -> 'Conversation': # Compute representativeness result._compute_repness() - + + # Compute comment priorities (D12 / PR 11). Needs PCA + group_votes. + result._compute_comment_priorities() + # Compute participant info result._compute_participant_info() - + return result + + def _compute_comment_priorities(self) -> Dict[Any, float]: + """ + Compute per-tid comment priorities matching Clojure + `:comment-priorities` (conversation.clj:648-679). + + Per-tid: sum A/D/S across all groups → P = S - (A + D) → call + `priority_metric(is_meta, A, P, S, E)` where E is the comment + extremity computed from PCA. + + Stores the result on `self.comment_priorities` and also returns it. + TS server `nextComment.ts::getNextPrioritizedComment` consumes this + for weighted comment routing — pre-D12 Python emitted nothing, so + the server fell back to uniform random selection. + """ + if self.pca is None or self.rating_mat is None or self.rating_mat.empty: + self.comment_priorities = {} + return self.comment_priorities + + center = np.asarray(self.pca.get('center')) + comps = np.asarray(self.pca.get('comps')) + if center.size == 0 or comps.size == 0: + self.comment_priorities = {} + return self.comment_priorities + + # Comment projection + extremity (Clojure with-proj-and-extremtiy, + # conversation.clj:341-352). + cmnt_proj = pca_project_cmnts(center, comps) + extremity_arr = compute_comment_extremity(cmnt_proj) + + # Column order of `center`/`comps`/`extremity_arr` matches + # `self.rating_mat.columns` (PCA is computed on rating_mat). + tid_extremity = dict(zip(self.rating_mat.columns, extremity_arr)) + + # Per-group A/D/S aggregation. `_compute_group_votes` returns + # {str(gid): {'n-members': N, 'votes': {tid: {A, D, S}}}}. S includes + # PASS (line ~1222: `np.sum(~np.isnan(votes))`), matching Clojure. + group_votes = self._compute_group_votes() + + priorities: Dict[Any, float] = {} + for tid in self.rating_mat.columns: + A_total = 0 + D_total = 0 + S_total = 0 + for gv_data in group_votes.values(): + votes_for_tid = gv_data.get('votes', {}).get( + tid, {'A': 0, 'D': 0, 'S': 0}) + A_total += votes_for_tid.get('A', 0) + D_total += votes_for_tid.get('D', 0) + S_total += votes_for_tid.get('S', 0) + # Clojure: P = S - (A + D) (conversation.clj:661). + P_total = S_total - (A_total + D_total) + E = float(tid_extremity.get(tid, 0)) + is_meta = tid in self.meta_tids + # Match key type with the rest of the codebase (int when possible). + try: + tid_key = int(tid) + except (ValueError, TypeError): + tid_key = tid + priorities[tid_key] = float(priority_metric( + is_meta, A_total, P_total, S_total, E)) + + self.comment_priorities = priorities + return priorities def get_summary(self) -> Dict[str, Any]: """ @@ -1731,12 +1891,15 @@ def numpy_to_list(arr): # a list-of-dicts format that would break server/src/report.ts, # server/src/utils/pca.ts, and client-participation-alpha consumers. - # Add empty consensus structure for compatibility - result['consensus'] = { - 'agree': [], - 'disagree': [], - 'comment-stats': {} - } + # Surface D11 consensus comments (Clojure parity: client-report's Majority + # view consumes result['consensus']). Pre-Investigation-B this block was + # hardcoded empty, which silently zeroed the Majority view regardless of + # the D11 selection. Falls back to the empty shape when repness is missing + # or did not produce a consensus_comments dict (older blobs, no-group convs). + result['consensus'] = ( + self.repness.get('consensus_comments', {'agree': [], 'disagree': []}) + if self.repness else {'agree': [], 'disagree': []} + ) # Add math_tick value current_time = int(time.time()) @@ -2272,12 +2435,14 @@ def float_to_decimal(obj): } result['pca'] = float_to_decimal(pca_data) - # Add consensus structure - result['consensus'] = { - 'agree': [], - 'disagree': [], - 'comment_stats': {} - } + # Surface D11 consensus comments (Clojure parity). Pre-Investigation-B + # this block was hardcoded empty, so the DynamoDB blob never carried the + # D11 dict even when repness produced one. Falls back to the empty shape + # when repness is missing or didn't produce consensus_comments. + result['consensus'] = ( + self.repness.get('consensus_comments', {'agree': [], 'disagree': []}) + if self.repness else {'agree': [], 'disagree': []} + ) # Add math_tick value current_time = int(time.time()) diff --git a/delphi/polismath/database/dynamodb.py b/delphi/polismath/database/dynamodb.py index 3d74433de..9d4ddcae3 100644 --- a/delphi/polismath/database/dynamodb.py +++ b/delphi/polismath/database/dynamodb.py @@ -300,6 +300,13 @@ def write_conversation(self, conv) -> bool: if analysis_table: if dynamo_data: # Use pre-formatted data + # D11 cascade fix (Investigation B, Site 1): source the FULL + # consensus dict from repness.consensus_comments, preserving + # both `agree` and `disagree` lists. The old code dropped + # `disagree` entirely (`.get('consensus', {}).get('agree', [])`). + consensus_comments = dynamo_data.get('repness', {}).get( + 'consensus_comments', {'agree': [], 'disagree': []} + ) analysis_table.put_item(Item={ 'zid': zid, 'math_tick': math_tick, @@ -308,7 +315,7 @@ def write_conversation(self, conv) -> bool: 'comment_count': dynamo_data.get('comment_count', 0), 'group_count': dynamo_data.get('group_count', 0), 'pca': dynamo_data.get('pca', {}), - 'consensus_comments': dynamo_data.get('consensus', {}).get('agree', []) + 'consensus_comments': consensus_comments }) else: # Legacy format @@ -321,11 +328,21 @@ def write_conversation(self, conv) -> bool: } # Replace floats with Decimal for DynamoDB pca_data = self._replace_floats_with_decimals(pca_data) - - # Create the analysis record with Decimal conversion - consensus_comments = self._numpy_to_list(conv.consensus) if hasattr(conv, 'consensus') else [] + + # D11 cascade fix (Investigation B, Site 2): the old code + # sourced from `conv.consensus`, which is always `[]` post-D11 + # (the attribute was deprecated). Source from + # `conv.repness['consensus_comments']` instead — the new shape + # is `{'agree': [...], 'disagree': [...]}`. + if hasattr(conv, 'repness') and conv.repness: + consensus_comments = conv.repness.get( + 'consensus_comments', {'agree': [], 'disagree': []} + ) + else: + consensus_comments = {'agree': [], 'disagree': []} + consensus_comments = self._numpy_to_list(consensus_comments) consensus_comments = self._replace_floats_with_decimals(consensus_comments) - + analysis_table.put_item(Item={ 'zid': zid, 'math_tick': math_tick, @@ -840,7 +857,13 @@ def read_math_by_tick(self, zid: str, math_tick: int) -> Dict[str, Any]: } # Set consensus - result['consensus'] = analysis.get('consensus_comments', []) + # D11 cascade fix (Investigation B, Site 3): default to the + # new dict shape `{'agree': [], 'disagree': []}` rather than + # the obsolete empty list `[]`, so downstream consumers + # always receive a uniformly-shaped value. + result['consensus'] = analysis.get( + 'consensus_comments', {'agree': [], 'disagree': []} + ) # 2. Get groups data groups_table = self.tables.get('Delphi_KMeansClusters') diff --git a/delphi/polismath/pca_kmeans_rep/pca.py b/delphi/polismath/pca_kmeans_rep/pca.py index cf6291975..696ffdfdb 100644 --- a/delphi/polismath/pca_kmeans_rep/pca.py +++ b/delphi/polismath/pca_kmeans_rep/pca.py @@ -123,5 +123,66 @@ def pca_project_dataframe(df: pd.DataFrame, # Create fallback projections (all zeros) n_proj = min(n_cols, 2) proj_dict = {pid: np.zeros(n_proj) for pid in df.index} - - return pca_results, proj_dict \ No newline at end of file + + return pca_results, proj_dict + + +# ============================================================================= +# D12: Comment projection / extremity (Clojure parity) +# ============================================================================= +# +# Port of Clojure `pca-project-cmnts` (math/src/polismath/math/pca.clj:167-178) +# and the extremity step from `with-proj-and-extremtiy` +# (math/src/polismath/math/conversation.clj:341-352). + +def pca_project_cmnts(center: np.ndarray, comps: np.ndarray) -> np.ndarray: + """ + Project each comment into the 2D PCA space. + + Clojure (`pca-project-cmnts`, pca.clj:167-178) calls + `sparsity-aware-project-ptpts` on a synthetic vote matrix where row `i` + has value `-1` at column `i` and `nil` everywhere else. + + For comment `i`, the sparsity-aware reduce (pca.clj:134-157) collapses to: + n_votes = 1 (only column i is non-nil) + p1 = (-1 - center[i]) * pc1[i] + p2 = (-1 - center[i]) * pc2[i] + scale = sqrt(n_cmnts / max(1, 1)) = sqrt(n_cmnts) + Final row: + proj[i] = sqrt(n_cmnts) * (-1 - center[i]) * [pc1[i], pc2[i]] + = -sqrt(n_cmnts) * (1 + center[i]) * [pc1[i], pc2[i]] + + Args: + center: PCA center (column means), shape (n_cmnts,). + comps: PCA components, shape (n_components, n_cmnts). Typically + n_components == 2. + + Returns: + Array of shape (n_cmnts, n_components) — projection per comment, in + the same column order as `center` / `comps`. + """ + n_cmnts = len(center) + if n_cmnts == 0: + return np.zeros((0, comps.shape[0] if comps.ndim == 2 else 0)) + scale = np.sqrt(n_cmnts) + coefs = -scale * (1.0 + center) # shape (n_cmnts,) + return coefs[:, None] * comps.T # shape (n_cmnts, n_components) + + +def compute_comment_extremity(cmnt_proj: np.ndarray) -> np.ndarray: + """ + Per-comment extremity = L2 norm of each projection row. + + Clojure parity: `with-proj-and-extremtiy` (conversation.clj:347-349) maps + `matrix/length` over each row of `pca-project-cmnts`. `matrix/length` is + Euclidean norm. + + Args: + cmnt_proj: shape (n_cmnts, n_components). + + Returns: + Shape (n_cmnts,) — extremity per comment. + """ + if cmnt_proj.size == 0: + return np.zeros(0) + return np.linalg.norm(cmnt_proj, axis=1) \ No newline at end of file diff --git a/delphi/polismath/pca_kmeans_rep/repness.py b/delphi/polismath/pca_kmeans_rep/repness.py index bbc5fb1f4..acd955a6c 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). + + Scalar equivalent (the formula this implements element-wise): - Formula: 2 * sqrt(n+1) * ((succ+1)/(n+1) - 0.5) + def prop_test(succ, n): + return 2 * sqrt(n + 1) * ((succ + 1) / (n + 1) - 0.5) - See prop_test() docstring for derivation and rationale. + 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): + + 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 - Matches Clojure's stats/two-prop-test (stats.clj:18-33). - See two_prop_test() scalar version for formula details. + +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,159 +344,110 @@ 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: - """ - Select representative comments for a single group from a 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). - DataFrame-native version of select_rep_comments(). - 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 +def passes_by_test(s: Dict[str, Any]) -> bool: + """ + Clojure passes-by-test? (repness.clj:165-170). - Returns: - DataFrame of selected representative comments + 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))) """ - 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 - - -def select_consensus_comments_df(stats_df: pd.DataFrame, - n_groups: int) -> List[Dict[str, Any]]: + 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: """ - Select consensus comments from DataFrame. + Clojure beats-best-by-test? (repness.clj:133-139). - Args: - stats_df: DataFrame with all (group, comment) statistics - n_groups: Number of groups + True if `s` has a more-representative max(rat, rdt) than `current_best_z`, + OR if there is no current best yet. Strict `>` (Clojure: `>`). - Returns: - List of consensus comment dicts + (or (nil? current-best-z) + (> (max rat rdt) current-best-z)) """ - if stats_df.empty: - return [] - - # Group by comment and check if all groups have high agreement - stats_reset = stats_df.reset_index() - comment_stats = stats_reset.groupby('comment').agg( - min_pa=('pa', 'min'), - avg_pa=('pa', 'mean'), - group_count=('group_id', 'count') - ) + if current_best_z is None: + return True + return max(s['rat'], s['rdt']) > current_best_z - # Filter to comments where all groups agree (pa > 0.6 for all) - # and present in all groups - consensus = comment_stats[ - (comment_stats['min_pa'] > 0.6) & - (comment_stats['group_count'] == n_groups) - ].copy() - - if consensus.empty: - return [] - - # Sort by average agreement and take top 2 - consensus = consensus.nlargest(2, 'avg_pa') - - # Convert to list of dicts using _stats_row_to_dict for legacy format - result = [] - for comment_id in consensus.index: - comment_rows = stats_reset[stats_reset['comment'] == comment_id] - # Convert each row to legacy dict format - stats_list = [_stats_row_to_dict(row) for _, row in comment_rows.iterrows()] - result.append({ - 'comment_id': comment_id, - 'avg_agree': consensus.loc[comment_id, 'avg_pa'], - 'repful': 'consensus', - 'stats': stats_list - }) - return result +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: -def _stats_row_to_dict(row: pd.Series) -> Dict[str, Any]: - """Convert a stats DataFrame row to the legacy dict format.""" - return { + 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). + """ + 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']), @@ -907,11 +463,304 @@ def _stats_row_to_dict(row: pd.Series) -> Dict[str, Any]: 'rdt': row['rdt'], 'agree_metric': row['agree_metric'], 'disagree_metric': row['disagree_metric'], - 'repful': row['repful'], + 'repful': repful, + } + if is_best_agree: + finalized['best_agree'] = True + finalized['n_agree'] = int(row['na']) + return finalized + + +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, 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: + `(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 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 + + +# ============================================================================= +# D11: Consensus comment selection (Clojure parity) +# ============================================================================= +# +# Ports of Clojure's `consensus-stats` and `select-consensus-comments` +# (math/src/polismath/math/repness.clj:284-323). +# +# Conceptually different from rep-comment selection: consensus stats are +# computed over the FULL conversation (no group split — `add-comparitive-stats` +# is NOT called). Two independent top-5 lists are then built — one for "agree +# consensus" (pa > 0.5 AND z-sig-90 on pat) ordered by `pa * pat`, one for +# "disagree consensus" (pd > 0.5 AND z-sig-90 on pdt) ordered by `pd * pdt`. + +def consensus_stats_df(vote_matrix_df: pd.DataFrame, + mod_out: Optional[Iterable[int]] = None + ) -> pd.DataFrame: + """ + Compute per-comment consensus stats across the whole conversation. + + Vectorized port of Clojure `consensus-stats` (repness.clj:284-290). Unlike + `compute_group_comment_stats_df`, no group split and no `ra/rd/rat/rdt` + (Clojure's `add-comparitive-stats` is not called here). + + Args: + vote_matrix_df: Wide-format vote matrix (participants × comments). + Values in {AGREE, DISAGREE, PASS, NaN}. + mod_out: Optional iterable of tids to exclude. Belt-and-braces with + D15 column-zeroing: moderated-out columns auto-fail the `pa > 0.5` + filter downstream (na=nd=0 → pa=pd=0.5), but the explicit filter + matches Clojure's behaviour (repness.clj:296). + + Returns: + DataFrame indexed by tid with columns [na, nd, ns, pa, pd, pat, pdt]. + """ + # Per-column counts. `vote_matrix_df` may have NaN for unvoted cells; + # those count as neither agree nor disagree. + na = (vote_matrix_df == AGREE).sum(axis=0).astype(int) + nd = (vote_matrix_df == DISAGREE).sum(axis=0).astype(int) + # ns counts all non-nil votes (incl. PASS) — Clojure parity, repness.clj:56-61. + ns = vote_matrix_df.notna().sum(axis=0).astype(int) + + df = pd.DataFrame({'na': na, 'nd': nd, 'ns': ns}) + df.index.name = 'tid' + + # pa, pd with PSEUDO_COUNT smoothing. + # Scalar equivalent: pa = (na + 1) / (ns + 2), pd = (nd + 1) / (ns + 2) + df['pa'] = (df['na'] + PSEUDO_COUNT / 2) / (df['ns'] + PSEUDO_COUNT) + df['pd'] = (df['nd'] + PSEUDO_COUNT / 2) / (df['ns'] + PSEUDO_COUNT) + zero_mask = df['ns'] == 0 + df.loc[zero_mask, 'pa'] = 0.5 + df.loc[zero_mask, 'pd'] = 0.5 + + # Proportion-test z-scores. + df['pat'] = prop_test_vectorized(df['na'], df['ns']) + df['pdt'] = prop_test_vectorized(df['nd'], df['ns']) + + if mod_out: + mod_out_set = set(mod_out) + df = df[~df.index.isin(mod_out_set)] + + return df + + +def select_consensus_comments_df( + cons_stats: pd.DataFrame, +) -> Dict[str, List[Dict[str, Any]]]: + """ + Select consensus comments (Clojure parity). + + Port of Clojure `select-consensus-comments` (repness.clj:293-323). Returns + two independent top-5 lists — one for agree consensus, one for disagree + consensus. + + Filters and ordering: + - Agree: `pa > 0.5 AND z-sig-90(pat)`, sorted desc by `am = pa * pat`. + - Disagree: `pd > 0.5 AND z-sig-90(pdt)`, sorted desc by `dm = pd * pdt`. + + With PSEUDO_COUNT smoothing the constraint `pa + pd = 1` is exact (na+nd=ns + after smoothing), so `pa > 0.5 ⟺ pd < 0.5` — the same tid cannot appear in + both lists. + + Args: + cons_stats: DataFrame indexed by tid with cols [na, nd, ns, pa, pd, + pat, pdt], as produced by `consensus_stats_df`. + + Returns: + Dict shape `{'agree': [entries], 'disagree': [entries]}`. Each entry + is `{comment_id, n_success, n_trials, p_success, p_test}` (Python + convention key naming per S1; math-blob alignment with Clojure's + hyphenated keys is a future PR). + """ + if cons_stats.empty: + return {'agree': [], 'disagree': []} + + df = cons_stats.copy() + df['am'] = df['pa'] * df['pat'] + df['dm'] = df['pd'] * df['pdt'] + + agree_filter = (df['pa'] > 0.5) & (df['pat'] > Z_90) + disagree_filter = (df['pd'] > 0.5) & (df['pdt'] > Z_90) + + agree_top = df[agree_filter].nlargest(5, 'am') + disagree_top = df[disagree_filter].nlargest(5, 'dm') + + def _agree_entry(tid: Any, row: pd.Series) -> Dict[str, Any]: + return { + 'comment_id': int(tid), + 'n_success': int(row['na']), + 'n_trials': int(row['ns']), + 'p_success': float(row['pa']), + 'p_test': float(row['pat']), + } + + def _disagree_entry(tid: Any, row: pd.Series) -> Dict[str, Any]: + return { + 'comment_id': int(tid), + 'n_success': int(row['nd']), + 'n_trials': int(row['ns']), + 'p_success': float(row['pd']), + 'p_test': float(row['pdt']), + } + + return { + 'agree': [_agree_entry(tid, row) for tid, row in agree_top.iterrows()], + 'disagree': [_disagree_entry(tid, row) for tid, row in disagree_top.iterrows()], } -def conv_repness(vote_matrix_df: pd.DataFrame, group_clusters: List[Dict[str, Any]]) -> Dict[str, Any]: +def conv_repness(vote_matrix_df: pd.DataFrame, + group_clusters: List[Dict[str, Any]], + mod_out: Optional[Iterable[int]] = None, + ) -> Dict[str, Any]: """ Calculate representativeness for all comments and groups. @@ -921,19 +770,23 @@ def conv_repness(vote_matrix_df: pd.DataFrame, group_clusters: List[Dict[str, An vote_matrix_df: pd.DataFrame of matrix of votes (participants × comments) Values should be AGREE (1), DISAGREE (-1), PASS (0), or NaN (unvoted) group_clusters: List of group clusters, each with 'id' and 'members' + mod_out: Optional iterable of tids to exclude (moderated-out comments). + Forwarded to `select_rep_comments_df` and `consensus_stats_df`. + See `Conversation.mod_out_tids`. Returns: Dictionary with representativeness data for each group: - comment_ids: list of comment IDs - group_repness: dict mapping group_id -> list of representative comments - - consensus_comments: list of consensus comments + - consensus_comments: dict `{'agree': [...], 'disagree': [...]}` after + D11 (was a flat list pre-D11; Clojure parity per repness.clj:322-323) - comment_repness: list of all comment repness data """ # Create empty-result structure in case we need to return early empty_result = { 'comment_ids': vote_matrix_df.columns.tolist(), 'group_repness': {group['id']: [] for group in group_clusters}, - 'consensus_comments': [], + 'consensus_comments': {'agree': [], 'disagree': []}, 'comment_repness': [] } @@ -998,25 +851,27 @@ 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). Forward `mod_out` from conv_repness (D11 + # added this kwarg). + result['group_repness'][group_id] = _assemble_rep_comments( + group_stats, mod_out=mod_out) except Exception as e: print(f"Error selecting representative comments for group {group_id}: {e}") result['group_repness'][group_id] = [] - # Add consensus comments if there are multiple groups + # Consensus comments (D11 / PR 9). Whole-conversation stats, not per-group. + # Clojure runs this unconditionally (conversation.clj:706-709) — no + # `len(group_clusters) > 1` guard. try: - if len(group_clusters) > 1: - result['consensus_comments'] = select_consensus_comments_df( - stats_df, len(group_clusters) - ) - else: - result['consensus_comments'] = [] + cons_stats = consensus_stats_df(vote_matrix_df, mod_out=mod_out) + result['consensus_comments'] = select_consensus_comments_df(cons_stats) except Exception as e: print(f"Error selecting consensus comments: {e}") - result['consensus_comments'] = [] + result['consensus_comments'] = {'agree': [], 'disagree': []} return result diff --git a/delphi/tests/test_discrepancy_fixes.py b/delphi/tests/test_discrepancy_fixes.py index 9e7390d9e..5f6a956aa 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,11 +40,28 @@ 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, + # D11 consensus helpers (PR 9) + consensus_stats_df, + select_consensus_comments_df, ) +from polismath.pca_kmeans_rep.pca import ( + pca_project_cmnts, + compute_comment_extremity, +) +from polismath.conversation.conversation import ( + importance_metric, + priority_metric, + META_PRIORITY, +) +from polismath.utils.general import AGREE, DISAGREE from polismath.regression import get_dataset_files, get_blob_variants from polismath.regression.datasets import discover_datasets from conftest import _get_requested_datasets, make_dataset_params, parse_dataset_blob_id @@ -618,7 +636,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 +722,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 +778,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 +845,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 +959,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 +1037,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 +1120,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 +1159,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 # ============================================================================ @@ -1158,29 +1646,213 @@ class TestD11ConsensusSelection: Clojure uses per-comment pa > 0.5, top 5 agree + 5 disagree with z-test scores """ - @pytest.mark.xfail(reason="D11: Different consensus selection logic than Clojure") + @pytest.mark.xfail(strict=False, + reason="After ns-PASS fix (2026-06-11), 3/4 dataset variants " + "(vw-incremental, vw-cold_start, biodiversity-cold_start) match " + "Clojure exactly. biodiversity-incremental still mismatches on " + "the disagree side (likely residual upstream PCA/KMeans " + "group-membership divergence affecting which participants are " + "in-conv at the incremental step). Tracked separately — see " + "journal entry 2026-06-11.") def test_consensus_matches_clojure(self, conv, clojure_blob, dataset_name): - """Consensus comments should match Clojure's selection.""" + """Consensus selection should match Clojure on cold_start. + + After D11 (PR 9), Python's `consensus_comments` is a dict + `{'agree': [...], 'disagree': [...]}` mirroring Clojure's shape. + Consensus stats are whole-conversation (no group split), so unlike + rep-comments this is NOT affected by upstream PCA/KMeans + group-membership divergence. The ns-PASS divergence + (DISCOVERY 2026-06-11) was fixed by switching `ns` from `na + nd` + to `notna().sum()` — matches Clojure `(count (filter identity ...))` + in repness.clj:56-61. + """ clj_consensus = clojure_blob.get('consensus', {}) if not clj_consensus: pytest.skip("No consensus in Clojure blob") - # Clojure consensus has 'agree' and 'disagree' keys clj_agree_tids = set(e['tid'] for e in clj_consensus.get('agree', [])) clj_disagree_tids = set(e['tid'] for e in clj_consensus.get('disagree', [])) clj_all = clj_agree_tids | clj_disagree_tids - py_consensus = conv.repness.get('consensus_comments', []) if conv.repness else [] - py_tids = set(int(c['comment_id']) for c in py_consensus) + py_consensus = (conv.repness.get('consensus_comments', {}) + if conv.repness else {}) + py_agree_tids = set(int(c['comment_id']) + for c in py_consensus.get('agree', [])) + py_disagree_tids = set(int(c['comment_id']) + for c in py_consensus.get('disagree', [])) + py_all = py_agree_tids | py_disagree_tids + + print(f"[{dataset_name}] Consensus Clojure: " + f"agree={sorted(clj_agree_tids)}, " + f"disagree={sorted(clj_disagree_tids)}") + print(f"[{dataset_name}] Consensus Python: " + f"agree={sorted(py_agree_tids)}, " + f"disagree={sorted(py_disagree_tids)}") + overlap = len(clj_all & py_all) + print(f"[{dataset_name}] Consensus overlap: {overlap}/{len(clj_all)}") + + check.equal(py_agree_tids, clj_agree_tids, + f"Agree consensus mismatch") + check.equal(py_disagree_tids, clj_disagree_tids, + f"Disagree consensus mismatch") - print(f"[{dataset_name}] Consensus: Clojure agree={sorted(clj_agree_tids)}, disagree={sorted(clj_disagree_tids)}") - print(f"[{dataset_name}] Consensus: Python={sorted(py_tids)}") - overlap = len(clj_all & py_tids) - print(f"[{dataset_name}] Consensus overlap: {overlap}/{len(clj_all)}") +class TestD11ConsensusStatsDf: + """`consensus_stats_df` — whole-conversation per-comment stats (no group split).""" + + @staticmethod + def _vote_matrix(per_comment_votes): + """Helper: build a vote matrix from {tid: [vote_per_participant]}.""" + return pd.DataFrame(per_comment_votes) + + def test_basic_counts(self): + """na/nd/ns counted correctly across all participants.""" + # 5 participants, 3 comments + # tid 1: 4 agrees, 1 disagree → na=4, nd=1, ns=5 + # tid 2: 2 agrees, 3 disagrees → na=2, nd=3, ns=5 + # tid 3: 1 agree, 2 disagrees, 2 NaN (pass/unvoted) → na=1, nd=2, ns=3 + votes = pd.DataFrame({ + 1: [AGREE, AGREE, AGREE, AGREE, DISAGREE], + 2: [AGREE, AGREE, DISAGREE, DISAGREE, DISAGREE], + 3: [AGREE, DISAGREE, DISAGREE, np.nan, np.nan], + }) + df = consensus_stats_df(votes) + assert df.loc[1, 'na'] == 4 and df.loc[1, 'nd'] == 1 and df.loc[1, 'ns'] == 5 + assert df.loc[2, 'na'] == 2 and df.loc[2, 'nd'] == 3 and df.loc[2, 'ns'] == 5 + assert df.loc[3, 'na'] == 1 and df.loc[3, 'nd'] == 2 and df.loc[3, 'ns'] == 3 + + def test_pseudocount_pa_pd(self): + """pa/pd use Beta(2,2) smoothing: (na+1)/(ns+2).""" + votes = pd.DataFrame({1: [AGREE, AGREE, AGREE, AGREE, DISAGREE]}) + df = consensus_stats_df(votes) + # na=4, ns=5 → pa = 5/7 ≈ 0.714 + assert abs(df.loc[1, 'pa'] - 5/7) < 1e-10 + # nd=1, ns=5 → pd = 2/7 ≈ 0.286 + assert abs(df.loc[1, 'pd'] - 2/7) < 1e-10 + + def test_ns_zero_uses_uninformative_prior(self): + """When ns=0 (no agree/disagree at all), pa=pd=0.5.""" + votes = pd.DataFrame({1: [np.nan, np.nan, np.nan]}) + df = consensus_stats_df(votes) + assert df.loc[1, 'pa'] == 0.5 + assert df.loc[1, 'pd'] == 0.5 + + def test_mod_out_filters_tids(self): + """`mod_out` removes tids from the output.""" + votes = pd.DataFrame({ + 1: [AGREE, AGREE, AGREE], + 2: [AGREE, AGREE, AGREE], + 3: [AGREE, AGREE, AGREE], + }) + df = consensus_stats_df(votes, mod_out={2}) + assert 1 in df.index + assert 2 not in df.index + assert 3 in df.index + + def test_ns_includes_pass_votes(self): + """Clojure parity: ns counts all non-nil votes incl. PASS (repness.clj:56-61).""" + votes = pd.DataFrame({ + 1: [AGREE, AGREE, DISAGREE, 0, 0], # 2A, 1D, 2P → ns=5 + }) + df = consensus_stats_df(votes) + assert df.loc[1, 'na'] == 2 + assert df.loc[1, 'nd'] == 1 + assert df.loc[1, 'ns'] == 5, f"ns should include PASS (Clojure parity); got {df.loc[1, 'ns']}" + + +class TestD11SelectConsensusBoundary: + """`select_consensus_comments_df` Clojure-parity boundaries.""" - check.equal(py_tids, clj_all, - f"Consensus mismatch: Python={sorted(py_tids)}, Clojure={sorted(clj_all)}") + @staticmethod + def _stats(rows): + """Helper: build a stats DataFrame from list of (tid, na, nd, ns, pa, pd, pat, pdt).""" + df = pd.DataFrame(rows, columns=['tid', 'na', 'nd', 'ns', 'pa', 'pd', 'pat', 'pdt']) + return df.set_index('tid') + + def test_empty_input_returns_empty_lists(self): + result = select_consensus_comments_df(pd.DataFrame(columns=['na', 'nd', 'ns', 'pa', 'pd', 'pat', 'pdt'])) + assert result == {'agree': [], 'disagree': []} + + def test_clear_agree_consensus(self): + """Comments with pa > 0.5 AND z-sig-90(pat) land in 'agree'.""" + stats = self._stats([ + (1, 9, 1, 10, 0.83, 0.17, 2.5, -2.5), # pa>0.5, pat z90 → agree + (2, 8, 2, 10, 0.75, 0.25, 2.0, -2.0), # agree + ]) + result = select_consensus_comments_df(stats) + agree_tids = [e['comment_id'] for e in result['agree']] + assert 1 in agree_tids and 2 in agree_tids + assert result['disagree'] == [] + + def test_clear_disagree_consensus(self): + """Comments with pd > 0.5 AND z-sig-90(pdt) land in 'disagree'.""" + stats = self._stats([ + (1, 1, 9, 10, 0.17, 0.83, -2.5, 2.5), # pd>0.5, pdt z90 → disagree + (2, 2, 8, 10, 0.25, 0.75, -2.0, 2.0), + ]) + result = select_consensus_comments_df(stats) + disagree_tids = [e['comment_id'] for e in result['disagree']] + assert 1 in disagree_tids and 2 in disagree_tids + assert result['agree'] == [] + + def test_divisive_no_consensus(self): + """Comments split ~50/50 with low z-scores → neither list populated.""" + stats = self._stats([ + (1, 5, 5, 10, 0.5, 0.5, 0.0, 0.0), + (2, 4, 6, 10, 0.42, 0.58, -0.4, 0.4), + ]) + result = select_consensus_comments_df(stats) + assert result['agree'] == [] + assert result['disagree'] == [] + + def test_top_5_cap_per_side(self): + """Each list capped at 5 entries.""" + # 7 high-agree comments + rows = [] + for i, am in enumerate([2.5, 2.3, 2.1, 1.9, 1.7, 1.5, 1.4]): + rows.append((i + 1, 9, 1, 10, 0.83, 0.17, am, -am)) + stats = self._stats(rows) + result = select_consensus_comments_df(stats) + assert len(result['agree']) == 5 + # Highest am at front: pa*pat = 0.83 * 2.5 = 2.075 + assert result['agree'][0]['comment_id'] == 1 + + def test_entry_keys_python_convention(self): + """Per-entry keys: comment_id, n_success, n_trials, p_success, p_test. + Python underscore convention (decision S1), not Clojure hyphens.""" + stats = self._stats([(1, 9, 1, 10, 0.83, 0.17, 2.5, -2.5)]) + result = select_consensus_comments_df(stats) + entry = result['agree'][0] + assert set(entry.keys()) == {'comment_id', 'n_success', 'n_trials', 'p_success', 'p_test'} + assert entry['comment_id'] == 1 + # For agree side, n_success = na, p_success = pa, p_test = pat + assert entry['n_success'] == 9 + assert entry['n_trials'] == 10 + assert abs(entry['p_success'] - 0.83) < 1e-10 + assert abs(entry['p_test'] - 2.5) < 1e-10 + + def test_disagree_entry_uses_d_keys(self): + """For disagree side, n_success = nd, p_success = pd, p_test = pdt.""" + stats = self._stats([(1, 1, 9, 10, 0.17, 0.83, -2.5, 2.5)]) + result = select_consensus_comments_df(stats) + entry = result['disagree'][0] + assert entry['n_success'] == 9 # = nd + assert abs(entry['p_success'] - 0.83) < 1e-10 # = pd + assert abs(entry['p_test'] - 2.5) < 1e-10 # = pdt + + def test_mutually_exclusive_lists(self): + """With PSEUDO_COUNT=2, pa + pd = 1 exactly (since na+nd=ns). So + pa > 0.5 ⟺ pd < 0.5 — the same tid cannot appear in both lists.""" + # Build several rows. For each, na+nd MUST equal ns (consensus_stats_df invariant). + stats = self._stats([ + (1, 7, 3, 10, 0.67, 0.33, 1.5, -1.5), # agree side + (2, 3, 7, 10, 0.33, 0.67, -1.5, 1.5), # disagree side + ]) + result = select_consensus_comments_df(stats) + agree_tids = {e['comment_id'] for e in result['agree']} + disagree_tids = {e['comment_id'] for e in result['disagree']} + assert agree_tids & disagree_tids == set(), \ + f"agree and disagree lists must be disjoint, got overlap {agree_tids & disagree_tids}" # ============================================================================ @@ -1194,9 +1866,24 @@ class TestD12CommentPriorities: Clojure computes priorities based on PCA extremity and importance. """ - @pytest.mark.xfail(reason="D12: Comment priorities not implemented in Python") + @pytest.mark.xfail(reason="D12.6 incremental divergence: Clojure cold_start has the truthy-0 " + "bug (all priorities = 49.0); Python mirrors that and matches on " + "cold_start. But Clojure INCREMENTAL doesn't exhibit the bug (varied " + "priorities), so Python's all-49 doesn't match incremental. Cold_start " + "would pass if parametrize allowed per-variant xfail. Once the Clojure " + "bug is fixed upstream, drop the Python mirror and this xfail.") def test_comment_priorities_exist(self, conv, clojure_blob, dataset_name): - """Python should produce comment-priorities matching Clojure.""" + """Python should produce comment-priorities matching Clojure. + + Per D12.6: Clojure's `(if 0 ...)` truthiness quirk means every tid + takes the meta branch, so Clojure cold_start priorities are all + META_PRIORITY^2 = 49.0 for vw/biodiversity. Python now mirrors this + bug + (priority_metric returns META_PRIORITY**2 unconditionally), so both + sides should yield identical all-constant 49.0. Spearman is not + meaningful when both sides have zero variance — we instead verify + the constant-value parity directly. + """ clj_priorities = clojure_blob.get('comment-priorities', {}) check.greater(len(clj_priorities), 0, f"Clojure has {len(clj_priorities)} comment priorities") @@ -1209,11 +1896,139 @@ def test_comment_priorities_exist(self, conv, clojure_blob, dataset_name): return py_priorities = conv.comment_priorities - # Compare rankings (Spearman correlation would be ideal, but check overlap first) - common_tids = set(str(k) for k in clj_priorities.keys()) & set(str(k) for k in py_priorities.keys()) - print(f"[{dataset_name}] Common priority tids: {len(common_tids)}/{len(clj_priorities)}") + # Normalize keys to int for comparison. + clj_p = {int(k): v for k, v in clj_priorities.items()} + py_p = {int(k): v for k, v in py_priorities.items()} + common_tids = set(clj_p.keys()) & set(py_p.keys()) + print(f"[{dataset_name}] Common priority tids: {len(common_tids)}/{len(clj_p)}") check.greater(len(common_tids), 0, "Should have common priority tids") + tids_sorted = sorted(common_tids) + clj_vals = [clj_p[t] for t in tids_sorted] + py_vals = [py_p[t] for t in tids_sorted] + clj_unique = set(clj_vals) + py_unique = set(py_vals) + print(f"[{dataset_name}] clj_vals sample: {clj_vals[:5]}, " + f"min={min(clj_vals)}, max={max(clj_vals)}, " + f"unique={len(clj_unique)}") + print(f"[{dataset_name}] py_vals sample: {py_vals[:5]}, " + f"min={min(py_vals)}, max={max(py_vals)}, " + f"unique={len(py_unique)}") + + # D12.6 Clojure-parity-bug mirror: both sides should return + # META_PRIORITY**2 = 49.0 for every tid. + META_PRIORITY_SQ = META_PRIORITY ** 2 + check.equal(len(clj_unique), 1, + f"Clojure priorities should be all-constant (bug); got {len(clj_unique)} unique") + check.equal(len(py_unique), 1, + f"Python priorities should be all-constant (bug mirror); got {len(py_unique)} unique") + if len(clj_unique) == 1: + (clj_const,) = clj_unique + check.almost_equal(clj_const, META_PRIORITY_SQ, abs=1e-9, + msg=f"Clojure constant priority should be META_PRIORITY**2={META_PRIORITY_SQ}") + if len(py_unique) == 1: + (py_const,) = py_unique + check.almost_equal(py_const, META_PRIORITY_SQ, abs=1e-9, + msg=f"Python constant priority should be META_PRIORITY**2={META_PRIORITY_SQ}") + + +class TestD12PCAProjectComments: + """`pca_project_cmnts` and `compute_comment_extremity` — Clojure parity.""" + + def test_pca_project_cmnts_shape(self): + """Output shape (n_cmnts, n_components).""" + center = np.array([0.1, 0.2, 0.3, 0.4]) + comps = np.array([[1.0, 0.0, 0.5, 0.5], + [0.0, 1.0, 0.5, -0.5]]) + proj = pca_project_cmnts(center, comps) + assert proj.shape == (4, 2) + + def test_pca_project_cmnts_formula(self): + """For comment i: proj[i] = -sqrt(n_cmnts) * (1 + center[i]) * [pc1[i], pc2[i]].""" + center = np.array([0.1, 0.2, 0.3, 0.4]) + comps = np.array([[1.0, 0.5, -0.5, 0.0], + [0.0, 0.5, 0.5, 1.0]]) + proj = pca_project_cmnts(center, comps) + n_cmnts = 4 + scale = np.sqrt(n_cmnts) + for i in range(n_cmnts): + expected = -scale * (1 + center[i]) * comps[:, i] + assert np.allclose(proj[i], expected), \ + f"proj[{i}] = {proj[i]} vs expected {expected}" + + def test_pca_project_cmnts_empty(self): + """Empty inputs return shape (0, n_comps).""" + center = np.zeros(0) + comps = np.zeros((2, 0)) + proj = pca_project_cmnts(center, comps) + assert proj.shape == (0, 2) + + def test_compute_comment_extremity_l2_norm(self): + """Extremity = L2 norm of each projection row.""" + cmnt_proj = np.array([[3.0, 4.0], + [0.0, 0.0], + [-1.0, 1.0]]) + ext = compute_comment_extremity(cmnt_proj) + assert np.allclose(ext, [5.0, 0.0, np.sqrt(2)]) + + def test_compute_comment_extremity_empty(self): + """Empty input → empty output.""" + ext = compute_comment_extremity(np.zeros((0, 2))) + assert ext.shape == (0,) + + +class TestD12PriorityMetrics: + """`importance_metric` and `priority_metric` — Clojure parity.""" + + def test_importance_metric_formula(self): + """`(1 - p) * (E + 1) * a` where p = (P+1)/(S+2), a = (A+1)/(S+2).""" + # Clojure ref values from conversation.clj:335: + # `(float (importance-metric 1 0 1 0))` — A=1, P=0, S=1, E=0 + # p = 1/3, a = 2/3, return = (2/3)*(1)*(2/3) = 4/9 ≈ 0.4444 + assert abs(importance_metric(1, 0, 1, 0) - 4 / 9) < 1e-10 + + def test_importance_metric_high_extremity_boosts(self): + """Higher extremity → higher importance.""" + baseline = importance_metric(5, 1, 8, 0.0) + boosted = importance_metric(5, 1, 8, 2.0) + assert boosted > baseline + + def test_priority_metric_meta_constant(self): + """Meta comments return META_PRIORITY^2 = 49 (Clojure parity).""" + # is_meta=True → inner = 7, return = 49 + assert priority_metric(True, 5, 2, 10, 1.5) == META_PRIORITY ** 2 + assert priority_metric(True, 0, 0, 0, 0) == META_PRIORITY ** 2 + + @pytest.mark.xfail(reason="Clojure parity bug mirror (D12.6): priority_metric always " + "returns META_PRIORITY**2 until upstream Clojure bug resolves. " + "Tests pin the semantically-correct formula and will pass again " + "when we revert the mirror.") + def test_priority_metric_non_meta_squared(self): + """Non-meta: return = (importance * (1 + 8*2^(-S/5)))^2.""" + # A=20, P=3, S=20, E=0 — ref from conversation.clj:337 + A, P, S, E = 20, 3, 20, 0 + imp = importance_metric(A, P, S, E) + decay = 1 + 8 * (2 ** (-S / 5)) + expected = (imp * decay) ** 2 + assert abs(priority_metric(False, A, P, S, E) - expected) < 1e-10 + + def test_priority_metric_decay_factor_lets_new_bubble_up(self): + """For low-S (new) comments, the decay factor is larger → priority boost.""" + # Two comments with identical importance metrics but different S. + # importance depends on A, P, S, E; to isolate the decay factor, + # pick A,P,E values that give same `(1 - (P+1)/(S+2)) * (E+1) * (A+1)/(S+2)`? + # Hard to isolate, so just test that the decay factor itself increases for low S. + new_decay = 1 + 8 * (2 ** (-1 / 5)) # S=1 + old_decay = 1 + 8 * (2 ** (-100 / 5)) # S=100 + assert new_decay > old_decay + assert new_decay > 1.0 + # Old comments fade toward 1 (no boost). + assert old_decay < 1.01 + + def test_meta_priority_constant_value(self): + """META_PRIORITY = 7 (Clojure conversation.clj:319).""" + assert META_PRIORITY == 7 + # ============================================================================ # D15 — Moderation Handling @@ -1620,36 +2435,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 +2456,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 +2518,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 +2534,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 +2559,120 @@ 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() + + 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)) - assert not mismatches, ( - f"[{dataset_name}] {len(mismatches)}/{total} p-success mismatches:\n" - + "\n".join(mismatches[:10])) + +class TestD11D12Serialization: + """Round-trip tests for the D11/D12 plumb-through in to_dict / to_dynamo_dict. + + Investigation B (2026-06-11) discovered that both serializers were hardcoding + ``result['consensus']`` to an empty dict regardless of + ``self.repness['consensus_comments']``, so the D11 consensus dict never + reached client-report's Majority view and never landed in the DynamoDB + math blob. ``comment_priorities`` (D12) was already conditionally plumbed + via ``hasattr/if`` guards; we lock that in with a regression test so a + future cleanup doesn't silently revert to the empty-default shape. + """ + + @staticmethod + def _make_conversation_with_repness(consensus_comments, priorities): + """Build a Conversation with just enough state to exercise the + serializers. Empty rating matrices and empty group_clusters mean the + rest of to_dict/to_dynamo_dict iterates over zero rows/cols (cheap) + while the consensus + priorities fields still flow through end-to-end. + """ + conv = Conversation(conversation_id='ztest-serialization') + conv.repness = { + 'comment_ids': [], + 'group_repness': {}, + 'comment_repness': [], + 'consensus_comments': consensus_comments, + } + conv.comment_priorities = priorities + return conv + + def test_to_dict_surfaces_consensus_comments(self): + """``to_dict()`` must surface ``self.repness['consensus_comments']`` into + ``result['consensus']``. Pre-fix this slot was hardcoded + ``{'agree': [], 'disagree': [], 'comment-stats': {}}`` and the D11 + selection was silently dropped on the floor.""" + consensus = { + 'agree': [ + {'tid': 1, 'n-success': 3, 'n-trials': 4, + 'p-success': 0.7, 'p-test': 1.5} + ], + 'disagree': [ + {'tid': 2, 'n-success': 2, 'n-trials': 5, + 'p-success': 0.42, 'p-test': 1.1} + ], + } + conv = self._make_conversation_with_repness(consensus, {}) + + result = conv.to_dict() + + assert result['consensus'] == consensus, ( + "to_dict() must plumb self.repness['consensus_comments'] into " + "result['consensus']; got " + repr(result['consensus'])) + + def test_to_dict_surfaces_comment_priorities(self): + """``to_dict()`` must surface ``self.comment_priorities`` (D12). This is + a regression lock: the field is currently conditionally plumbed via + ``hasattr/if``; a future cleanup must not revert to the hardcoded + empty default.""" + priorities = {1: 0.42, 2: 1.7, 3: 0.0} + conv = self._make_conversation_with_repness( + {'agree': [], 'disagree': []}, priorities) + + result = conv.to_dict() + + # The to_dict key uses underscore form (see line ~1706); no rename + # happens on the way out, unlike most Clojure-format fields. + assert 'comment_priorities' in result, ( + "to_dict() must emit 'comment_priorities' when " + "self.comment_priorities is populated; keys = " + + repr(sorted(result.keys()))) + assert result['comment_priorities'] == priorities + + def test_to_dynamo_dict_surfaces_both(self): + """``to_dynamo_dict()`` must surface BOTH consensus comments (D11) and + comment priorities (D12). The DynamoDB shape uses underscore keys + (``consensus``, ``comment_priorities``); the consensus inner shape + matches whatever ``self.repness['consensus_comments']`` holds + (Clojure-style ``agree``/``disagree`` lists).""" + consensus = { + 'agree': [ + {'tid': 11, 'n-success': 8, 'n-trials': 10, + 'p-success': 0.83, 'p-test': 2.1} + ], + 'disagree': [], + } + # Priorities use comment-id keys; the serializer coerces to int when + # possible and emits int values (see lines 2447-2456 of conversation.py). + priorities = {7: 1.5, 9: 0.25} + conv = self._make_conversation_with_repness(consensus, priorities) + + result = conv.to_dynamo_dict() + + assert result['consensus'] == consensus, ( + "to_dynamo_dict() must plumb self.repness['consensus_comments'] " + "into result['consensus']; got " + repr(result['consensus'])) + assert 'comment_priorities' in result, ( + "to_dynamo_dict() must emit 'comment_priorities' when " + "self.comment_priorities is populated; keys = " + + repr(sorted(result.keys()))) + # to_dynamo_dict coerces values to int via int(priority), so 1.5 -> 1 + # and 0.25 -> 0. We assert the post-coercion shape rather than the + # raw input to lock in what actually lands in DynamoDB. + assert result['comment_priorities'] == {7: 1, 9: 0} diff --git a/delphi/tests/test_dynamodb_consensus_roundtrip.py b/delphi/tests/test_dynamodb_consensus_roundtrip.py new file mode 100644 index 000000000..3f7ea9cbf --- /dev/null +++ b/delphi/tests/test_dynamodb_consensus_roundtrip.py @@ -0,0 +1,284 @@ +""" +Tests for D11 cascade fix in `delphi/polismath/database/dynamodb.py`. + +Investigation B (2026-06-11) found three sites in the DynamoDB writer/reader +that either dropped the new D11 `consensus_comments` dict shape +(`{'agree': [...], 'disagree': [...]}`) or defaulted to the obsolete empty +list. These tests assert that, given the new shape, the writer preserves +BOTH agree and disagree lists into the `Delphi_PCAResults` table, and that +the reader defaults to the new dict shape when no item is present. + +The boto3 `Table` resource is replaced with a `unittest.mock.MagicMock`, +so no DynamoDB process is required. +""" + +from unittest.mock import MagicMock + +import pytest + +from polismath.database.dynamodb import DynamoDBClient + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +_AGREE_ENTRY = { + 'comment_id': 1, + 'n_success': 3, + 'n_trials': 5, + 'p_success': 0.6, + 'p_test': 1.5, +} + +_DISAGREE_ENTRY = { + 'comment_id': 2, + 'n_success': 4, + 'n_trials': 5, + 'p_success': 0.8, + 'p_test': 2.0, +} + + +def _make_consensus_dict(): + """Return a fresh D11-shape consensus dict (copy per test).""" + return { + 'agree': [dict(_AGREE_ENTRY)], + 'disagree': [dict(_DISAGREE_ENTRY)], + } + + +class _StubConversation: + """Minimal Conversation-like stub for the writer's legacy branch. + + The writer only touches a handful of attributes for the PCAResults + write path, so we keep this stub deliberately tiny. The DynamoDB + `Delphi_PCAResults` write is independent of group_clusters / + comment_priorities / etc. — those affect other tables we are not + exercising here. + """ + + def __init__(self, repness): + self.conversation_id = 42 + self.participant_count = 10 + self.comment_count = 3 + self.group_clusters = [] + self.pca = {} + self.repness = repness + # `consensus` attribute is intentionally NOT set — Site 2 must + # source from `self.repness['consensus_comments']`, not from + # the deprecated `self.consensus` attribute. + + +def _client_with_pca_results_only(): + """Build a DynamoDBClient with ONLY the PCAResults table mocked. + + All other tables are None so the writer short-circuits early at each + later step. This keeps the test focused on the consensus write site. + """ + client = DynamoDBClient() + pca_results_table = MagicMock(name='Delphi_PCAResults') + client.tables = { + 'Delphi_PCAConversationConfig': None, + 'Delphi_PCAResults': pca_results_table, + 'Delphi_KMeansClusters': None, + 'Delphi_CommentRouting': None, + 'Delphi_RepresentativeComments': None, + 'Delphi_ParticipantProjections': None, + } + return client, pca_results_table + + +# --------------------------------------------------------------------------- +# Site 1 — `dynamo_data` branch (preferred path) +# --------------------------------------------------------------------------- + +class TestSite1DynamoDataBranch: + """When `conv.to_dynamo_dict()` returns the new shape, both lists land.""" + + def test_writes_both_agree_and_disagree(self): + client, pca_results_table = _client_with_pca_results_only() + + # Stub the conversation so the writer takes the `dynamo_data` branch. + conv = MagicMock(name='Conversation') + conv.conversation_id = 42 + conv.to_dynamo_dict.return_value = { + 'participant_count': 10, + 'comment_count': 3, + 'group_count': 0, + 'pca': {}, + 'math_tick': 30000, + 'repness': { + 'comment_repness': [], + 'consensus_comments': _make_consensus_dict(), + }, + } + + ok = client.write_conversation(conv) + assert ok is True + + assert pca_results_table.put_item.called, \ + "Writer did not call put_item on Delphi_PCAResults" + item = pca_results_table.put_item.call_args.kwargs['Item'] + + written = item['consensus_comments'] + assert isinstance(written, dict), \ + f"Expected dict shape, got {type(written).__name__}: {written!r}" + assert 'agree' in written, f"Missing 'agree' key: {written!r}" + assert 'disagree' in written, f"Missing 'disagree' key: {written!r}" + assert written['agree'] == [_AGREE_ENTRY] + assert written['disagree'] == [_DISAGREE_ENTRY] + + def test_missing_repness_uses_dict_default(self): + """No repness in dynamo_data → empty dict, not list, not crash.""" + client, pca_results_table = _client_with_pca_results_only() + + conv = MagicMock(name='Conversation') + conv.conversation_id = 42 + conv.to_dynamo_dict.return_value = { + 'participant_count': 10, + 'comment_count': 3, + 'group_count': 0, + 'pca': {}, + 'math_tick': 30000, + # repness intentionally absent + } + + ok = client.write_conversation(conv) + assert ok is True + + item = pca_results_table.put_item.call_args.kwargs['Item'] + assert item['consensus_comments'] == {'agree': [], 'disagree': []} + + +# --------------------------------------------------------------------------- +# Site 2 — legacy branch (no `to_dynamo_dict`) +# --------------------------------------------------------------------------- + +class TestSite2LegacyBranch: + """When `to_dynamo_dict` is absent, the writer sources from conv.repness.""" + + def test_writes_both_agree_and_disagree_from_repness(self): + client, pca_results_table = _client_with_pca_results_only() + + # Plain object without `to_dynamo_dict` → legacy branch. + conv = _StubConversation( + repness={'consensus_comments': _make_consensus_dict()} + ) + + ok = client.write_conversation(conv) + assert ok is True + + item = pca_results_table.put_item.call_args.kwargs['Item'] + written = item['consensus_comments'] + assert isinstance(written, dict), \ + f"Legacy branch produced non-dict: {type(written).__name__}: {written!r}" + assert set(written.keys()) >= {'agree', 'disagree'} + # _replace_floats_with_decimals converts floats to Decimal but + # preserves the list structure and integer fields. Confirm the + # comment_ids round-trip cleanly. + assert [c['comment_id'] for c in written['agree']] == [1] + assert [c['comment_id'] for c in written['disagree']] == [2] + + def test_missing_repness_uses_dict_default(self): + client, pca_results_table = _client_with_pca_results_only() + + # `repness` is None — writer must default to the dict shape. + conv = _StubConversation(repness=None) + + ok = client.write_conversation(conv) + assert ok is True + + item = pca_results_table.put_item.call_args.kwargs['Item'] + assert item['consensus_comments'] == {'agree': [], 'disagree': []} + + +# --------------------------------------------------------------------------- +# Site 3 — reader default +# --------------------------------------------------------------------------- + +class TestSite3ReaderDefault: + """`read_math_by_tick` must default consensus to the new dict shape.""" + + def test_missing_consensus_comments_returns_dict(self): + client = DynamoDBClient() + analysis_table = MagicMock(name='Delphi_PCAResults') + # Returned Item lacks `consensus_comments` entirely. + analysis_table.get_item.return_value = { + 'Item': { + 'participant_count': 10, + 'comment_count': 3, + 'pca': {'center': [], 'components': []}, + } + } + client.tables = { + 'Delphi_PCAResults': analysis_table, + 'Delphi_KMeansClusters': None, + 'Delphi_CommentRouting': None, + 'Delphi_RepresentativeComments': None, + 'Delphi_ParticipantProjections': None, + } + + result = client.read_math_by_tick('42', 30000) + assert result['consensus'] == {'agree': [], 'disagree': []} + + def test_present_consensus_comments_round_trip(self): + """When the stored Item already has the dict shape, it's returned verbatim.""" + client = DynamoDBClient() + analysis_table = MagicMock(name='Delphi_PCAResults') + stored = _make_consensus_dict() + analysis_table.get_item.return_value = { + 'Item': { + 'participant_count': 10, + 'comment_count': 3, + 'pca': {'center': [], 'components': []}, + 'consensus_comments': stored, + } + } + client.tables = { + 'Delphi_PCAResults': analysis_table, + 'Delphi_KMeansClusters': None, + 'Delphi_CommentRouting': None, + 'Delphi_RepresentativeComments': None, + 'Delphi_ParticipantProjections': None, + } + + result = client.read_math_by_tick('42', 30000) + assert result['consensus'] == stored + + +# --------------------------------------------------------------------------- +# Round-trip — write then read on the same in-memory store +# --------------------------------------------------------------------------- + +class TestRoundTrip: + """Smoke-test: writer output, fed back through the reader, preserves shape.""" + + def test_write_then_read_preserves_both_lists(self): + client, pca_results_table = _client_with_pca_results_only() + + # Record what the writer puts. + conv = MagicMock(name='Conversation') + conv.conversation_id = 42 + conv.to_dynamo_dict.return_value = { + 'participant_count': 10, + 'comment_count': 3, + 'group_count': 0, + 'pca': {}, + 'math_tick': 30000, + 'repness': { + 'comment_repness': [], + 'consensus_comments': _make_consensus_dict(), + }, + } + client.write_conversation(conv) + written_item = pca_results_table.put_item.call_args.kwargs['Item'] + + # Replay the written Item back through the reader. + pca_results_table.get_item.return_value = {'Item': written_item} + + result = client.read_math_by_tick('42', 30000) + consensus = result['consensus'] + assert isinstance(consensus, dict) + assert [c['comment_id'] for c in consensus['agree']] == [1] + assert [c['comment_id'] for c in consensus['disagree']] == [2] diff --git a/delphi/tests/test_legacy_clojure_regression.py b/delphi/tests/test_legacy_clojure_regression.py index 6ad2c6aed..b3c748abc 100644 --- a/delphi/tests/test_legacy_clojure_regression.py +++ b/delphi/tests/test_legacy_clojure_regression.py @@ -275,7 +275,11 @@ def test_group_clustering(self, conversation_data): check.is_true(result['overall_match'], f"Clustering should match Clojure output (distribution + membership)") - @pytest.mark.xfail(raises=AssertionError, strict=True, reason="D12: Comment priorities not yet implemented in Python") + @pytest.mark.xfail(raises=AssertionError, strict=False, + reason="D12 / D12.6 incremental: Python's Clojure-bug-mirror (all priorities = META_PRIORITY^2 = 49) " + "matches Clojure cold_start exactly (XPASS), but Clojure incremental has varied priorities, " + "so Python's all-49 doesn't match incremental. strict=False to allow both XPASS (cold_start) " + "and FAIL (incremental) without test failure. Restore strict=True once Clojure bug is fixed upstream.") def test_comment_priorities(self, conversation_data): """ Test that comment priorities match the Clojure implementation exactly. diff --git a/delphi/tests/test_legacy_repness_comparison.py b/delphi/tests/test_legacy_repness_comparison.py index e920aafde..6d4bd85c7 100644 --- a/delphi/tests/test_legacy_repness_comparison.py +++ b/delphi/tests/test_legacy_repness_comparison.py @@ -194,7 +194,15 @@ def _compare_results(self, py_results: Dict[str, Any], clj_results: Dict[str, An # Look for consensus comments if they exist if 'consensus-comments' in clj_repness: clj_consensus = clj_repness.get('consensus-comments', []) - py_consensus = py_results.get('consensus_comments', []) + # B2 fix (D11 sub-agent review): post-D11 (PR 9) Python's + # consensus_comments is a dict `{agree: [...], disagree: [...]}`, + # not a flat list. Flatten for the ID extraction below. + py_consensus_dict = py_results.get('consensus_comments', {}) + if isinstance(py_consensus_dict, dict): + py_consensus = (py_consensus_dict.get('agree', []) + + py_consensus_dict.get('disagree', [])) + else: + py_consensus = py_consensus_dict # legacy fallback # Extract comment IDs clj_consensus_ids = [str(c.get('comment-id', c.get('tid', c.get('comment_id', '')))) for c in clj_consensus] 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_pipeline_integrity.py b/delphi/tests/test_pipeline_integrity.py index 60d007e45..5f581cfe3 100644 --- a/delphi/tests/test_pipeline_integrity.py +++ b/delphi/tests/test_pipeline_integrity.py @@ -195,10 +195,15 @@ def test_full_pipeline(dataset_name: str) -> None: print(f" Agree: {comment.get('pa', 0):.2f}, Disagree: {comment.get('pd', 0):.2f}") print(f" Metrics: A={comment.get('agree_metric', 0):.2f}, D={comment.get('disagree_metric', 0):.2f}") - # Check consensus comments + # Check consensus comments. Post-D11 (PR 9), shape is + # `{'agree': [...], 'disagree': [...]}` matching Clojure. print("\n Consensus Comments:") - for i, comment in enumerate(updated_conv.repness.get('consensus_comments', [])): - print(f" - Comment {i+1}: ID {comment.get('comment_id')}, Avg Agree: {comment.get('avg_agree', 0):.2f}") + consensus = updated_conv.repness.get('consensus_comments', {}) + for side in ('agree', 'disagree'): + for i, comment in enumerate(consensus.get(side, [])): + print(f" - {side} #{i+1}: ID {comment.get('comment_id')}, " + f"p_success={comment.get('p_success', 0):.2f}, " + f"p_test={comment.get('p_test', 0):.2f}") else: print(" No representativeness results available") diff --git a/delphi/tests/test_repness_smoke.py b/delphi/tests/test_repness_smoke.py index 99834025a..154edcb3f 100644 --- a/delphi/tests/test_repness_smoke.py +++ b/delphi/tests/test_repness_smoke.py @@ -100,14 +100,16 @@ def test_repness_structure(self, dataset_name: str, conversation): assert 'repful' in comment # 'agree', 'disagree', or other type logger.debug(f"Group {group_id}: {len(comments)} representative comments") - # Check consensus comments if present + # Check consensus comments if present. Post-D11 (PR 9), shape is + # `{'agree': [...], 'disagree': [...]}` matching Clojure (repness.clj:322-323). if 'consensus_comments' in repness_results: consensus = repness_results['consensus_comments'] - logger.debug(f"Consensus comments: {len(consensus)}") + agree = consensus.get('agree', []) + disagree = consensus.get('disagree', []) + logger.debug(f"Consensus: {len(agree)} agree, {len(disagree)} disagree") - if len(consensus) > 0: - comment = consensus[0] - assert 'comment_id' in comment + for entry in agree + disagree: + assert 'comment_id' in entry logger.debug("✓ Representativeness structure validated") 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